Fix environment leakage in doctor_test #54478
Conversation
| }, overrides: <Type, Generator>{ | ||
| FeatureFlags: () => TestFeatureFlags(isWebEnabled: true), | ||
| FileSystem: () => MemoryFileSystem.test(), | ||
| ProcessManager: () => MockProcessManager(), |
There was a problem hiding this comment.
Does it make sense for the context override to check for the inverse (ProcessManager and no FileSystem)?
https://github.com/flutter/flutter/blob/master/packages/flutter_tools/test/src/context.dart#L64
There was a problem hiding this comment.
I guess the idea is if the process manager is mocked, then we shouldn't be hitting the file system at all?
There was a problem hiding this comment.
@jonahwilliams or @zanderso can you think of a reason?
There was a problem hiding this comment.
For context, I didn't change this because I knew for a fact it was hitting the file system, I just added it for consistency and since some of the other tests were hitting surprising globals.
There was a problem hiding this comment.
The real ProcessManager will certainly touch the real file system, subverting the intent of the FileSystem context override. The other way around doesn't have the same problem, but it could make sense to require always overriding both together just for consistency.
| // fail if the gen_snapshot binary is not present. | ||
| expect(testLogger.statusText, contains('No issues found!')); | ||
| expect(await FlutterValidatorDoctor().diagnose(verbose: false), isTrue); | ||
| // gen_snapshot is downloaded on demand, and the doctor should not |
There was a problem hiding this comment.
I guess? For context #52574.
Description
I was seeing these tests fail locally for me on master from a mac (it passes on CI). The reason was that some of these tests were using the real artifacts and file systems.
Tests
This PR modifies tests to properly mock the artifacts and use memory file systems.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read [Handling breaking changes].