[flutter_tools] standardize patterns for integration.shard#64980
[flutter_tools] standardize patterns for integration.shard#64980jonahwilliams merged 9 commits intoflutter:masterfrom
Conversation
|
|
||
| import '../src/common.dart'; | ||
|
|
||
| const FileSystem fileSystem = LocalFileSystem(); |
There was a problem hiding this comment.
Doesn't have to be in this PR, but should this be moved to the integration shard?
There was a problem hiding this comment.
I wasn't sure. It is definitely touching the real filesystem, but it is also more of an analysis check - I could see wanting it to remain fast.
There was a problem hiding this comment.
Does it run in parallel with other general shard tests?
There was a problem hiding this comment.
yes ... hopefully those aren't also writing to the filesytem...hmm
There was a problem hiding this comment.
we should probably just move this?
There was a problem hiding this comment.
i vote move them just to be safe and rule out the potential flaking.
| final List<String> allowedPath = <String>[ | ||
| globals.fs.path.join(flutterTools, 'lib', 'src', 'base', 'file_system.dart'), | ||
| fileSystem.path.join(flutterTools, 'test', 'general.shard', 'forbidden_imports_test.dart'), | ||
| fileSystem.path.join(flutterTools, 'test', 'integration.shard', 'test_utils.dart'), |
There was a problem hiding this comment.
would it be possible to allow the entire //test/integration.shard/ dir?
There was a problem hiding this comment.
we could - this sort of enforces using the existing test_utils, which could be helpful if we do want to change the file system in the future
There was a problem hiding this comment.
oh yeah, makes sense
…liams/flutter into remove_integration_test_globals
|
This does not affect google3 unless they are trying to run our integration tests. |
|
|
||
| import '../src/common.dart'; | ||
|
|
||
| /// The [FileSystem] for the integration test environment. |
There was a problem hiding this comment.
How about a test in forbidden_imports_test.dart that checks that this file isn't imported outside of the integration.shard subdir?
Description
Integration tests must only go through the real file system/process manager/platform. The global indirection makes this code harder to understand than if it directly referred to the concrete instances that are being used.
Update the integration shard to use a const instance of a LocalFIleSystem, LocalProcessManager, and LocalPlatform. Remove global usage and apply testWithoutContext.