Repository navigation
Differences in behaviour between real windows FS and Window-style in-memory FS #632
Description
I was moving some tests in Flutter over to the memory file system and hit some issues because of differences in behaviour. In the real implementation, if you use a forward slash in a path, it still resolves correctly. However in the memory file system it treats it as a different path.
The below (when run on Windows) prints:
true
false
import 'package:file/local.dart';
import 'package:file/memory.dart';
final localFs = new LocalFileSystem();
final memoryFs = new MemoryFileSystem(style: FileSystemStyle.windows);
main() {
[
new LocalFileSystem(),
new MemoryFileSystem(style: FileSystemStyle.windows),
].forEach((fs) {
// Create a file at test\test.txt
fs.currentDirectory = fs.systemTempDirectory;
fs.directory('test').createSync(recursive: true);
fs.file('test\\test.txt').createSync();
// Check if it exists using a forward slash
print(fs.file('test/test.txt').existsSync());
});
}I'll fix this in flutter by using the correct slashes, but I guess the intention is for these to behave the same where possible. It's possibly that https://github.com/google/file.dart/issues/56 would cover this, I'm not sure.
Activity
goderbauer commentedon Aug 29, 2018
On Windows it depends on which underlaying Windows API is used. Some can convert slashes, others don't. The only safe way is to use the correct slash on Windows everywhere and I think the memory file system should enforce that.
DanTup commentedon Aug 29, 2018
In that case, I wonder if Dart should be more aggressive (not just the memory provider). It seems weird if Dart's expected behaviour depends on whichever Windows API its implementation happens to use at a point in time (eg., internal refactors to use different APIs in Windows probably shouldn't have observable effects?).
But at least, having the memory provider enforce slashes would be a great change. If you move from the real FS to the Memory FS (as I just did) it would be better to get clear exceptions that are obvious to fix rather than subtle differences in behaviour (in this case, some code just claimed files didn't exist which had just been created - albeit with different path separators).
tvolkert commentedon Aug 29, 2018
/cc @zanderso
zanderso commentedon Aug 30, 2018
@bkonyi I won't have access to a Windows machine until next week, would you mind just trying to reproduce this using dart:io File directly?
DanTup commentedon Aug 30, 2018
Sure! I wasn't sure how much LocalFileSystem did and assumed it was the same. In this case, the behaviour does match anyway:
(Running on Windows 10 Pro 10.0.17134 Build 17134)
zanderso commentedon Aug 30, 2018
Oh, sorry, I misunderstood the issue. I had thought that the dart:io File was failing to handle both slash directions. We have lots of tests and code written in the SDK that rely on both directions working, so I think we're probably going to continue doing that.
DanTup commentedon Aug 30, 2018
That sounds good to me. My request is really that switching from real file system methods (or LocalFileSystem) to MemoryFileSystem doesn't introduce differences in behaviour (especially if they're subtle, silent and potentially only on one platform) :-)
tvolkert commentedon Aug 30, 2018
@DanTup would you be willing to write some tests in common_tests.dart that show this failure? We can skip them and then enable them when we fix the bug.
Change asset_bundle_package_font_test to memory file system (#21114)
Add tests that fail because of slash directions
DanTup commentedon Sep 5, 2018
I've opened a PR with some tests that pass if I use the same slash direction, but fail as-committed:
dart-archive/file.dart#115
I couldn't see an obvious way to skip them though.
Also, FWIW, I get 4 existing failures running on Windows, for example:
Let me know if I should open an issue with details.
Add tests that fail because of slash directions (#115)
tvolkert commentedon Mar 14, 2019
@cskau-g can you skip them in the
BUILDrule?tvolkert commentedon Mar 14, 2019
Yep, that sounds like a good solution.
Thanks!
Hixie commentedon Oct 9, 2019
Should this issue be closed?
dnfield commentedon Oct 9, 2019
The linked PR didn't fix the issue, it skipped the tests.
Hixie commentedon Oct 11, 2019
It's not clear to me that there is agreement that there actually is an issue.
DanTup commentedon Oct 14, 2019
My understanding from @zanderso's comment https://github.com/google/file.dart/issues/112#issuecomment-417269305 is that both slash directions should work using this, as they do in the real implementations. Having subtle differences in behaviour between your tests and what your users would actually see makes the tests less useful (in this case I had failing tests that should pass, but it wouldn't be hard to end up with the opposite).