Simplify Filesystem, introduce TestReaderWriter
#3873
Conversation
PR Health |
davidmorgan
force-pushed
the
in-memory-to-path
branch
2 times, most recently
from
1a6c8b1 to
f6b0048
Compare
davidmorgan
changed the title
Filesystem, introduce TestReaderWriter
Move `FilesystemCache` and `AssetPathProvider` out of `Filesystem` and into the `Reader` classes.
davidmorgan
force-pushed
the
in-memory-to-path
branch
from
f6b0048 to
cb8fc64
Compare
build/lib/src/state/filesystem.dart
Outdated
| : cache = cache ?? const PassthroughFilesystemCache(), | ||
| assets = {}; | ||
| @override | ||
| Future<bool> exists(String path) async => _files.containsKey(path); |
There was a problem hiding this comment.
it will be faster to have these not be async and return Future.value(whatnot).
| AssetId('example', 'web/initial.txt'), | ||
| 'initial', | ||
| ) | ||
| ..filesystem.writeAsStringSync( | ||
| ..testing.writeString( | ||
| AssetId('example', 'web/large.txt'), | ||
| List.filled(10000, 'large').join(''), |
There was a problem hiding this comment.
I know this is not new code, but isn't this just 'large' * 10000? (as in literally that code will give the same string).
There was a problem hiding this comment.
Yes it is, done :)
| @@ -1489,9 +1489,9 @@ void main() { | |||
| ); | |||
|
|
|||
| // Followup build with modified inputs. | |||
| var serializedGraph = | |||
| result.readerWriter.assets[makeAssetId('a|$assetGraphPath')]!; | |||
| result.readerWriter.assets.clear(); | |||
There was a problem hiding this comment.
What's the story on the clear? Here it's replaced by nothing --- below it's replaced by a delete call.
There was a problem hiding this comment.
In most of the test cases, like this one, the next run passes the same sources again, so the clear had no effect.
Only cases where the second run wants there to be fewer files actually needs a clear or delete, so now, that's what the tests do :)
| AssetId('example', 'web/initial.txt'), | ||
| 'initial', | ||
| ) | ||
| ..filesystem.writeAsStringSync( | ||
| ..testing.writeString( | ||
| AssetId('example', 'web/large.txt'), | ||
| List.filled(10000, 'large').join(''), |
There was a problem hiding this comment.
Yes it is, done :)
build/lib/src/state/filesystem.dart
Outdated
| : cache = cache ?? const PassthroughFilesystemCache(), | ||
| assets = {}; | ||
| @override | ||
| Future<bool> exists(String path) async => _files.containsKey(path); |
| @@ -1489,9 +1489,9 @@ void main() { | |||
| ); | |||
|
|
|||
| // Followup build with modified inputs. | |||
| var serializedGraph = | |||
| result.readerWriter.assets[makeAssetId('a|$assetGraphPath')]!; | |||
| result.readerWriter.assets.clear(); | |||
There was a problem hiding this comment.
In most of the test cases, like this one, the next run passes the same sources again, so the clear had no effect.
Only cases where the second run wants there to be fewer files actually needs a clear or delete, so now, that's what the tests do :)
For #3811, work in progress.
Move
FilesystemCacheout ofFilesystemtoFileBasedAssetReaderandInMemoryAssetReaderWriter.Filesystemnow just always directly reads/writes.Move
AssetPathProviderout ofFilesystemtoFileBasedAssetReaderandInMemoryAssetReaderWriter.Filesystemnow accesses files by path, with theReaderhandling the mapping from IDs to path. This removes the last big difference betweenInMemoryAssetReaderWriterandFileBasedAssetReader, which was that the in-memory version used did not map to paths, storing data by ID.Add
TestReaderWriterwithReaderWriterTestingmember that gives access to the internal state for testing;InMemoryAssetReaderWriterand itsFilesystemand otherAssetReaderStatemembers are now hidden from tests behind this test-specific API.