[flutter_tools] switch FakeCache to cache.test and NoopUsage to TestUsage #76802
Conversation
flutter-dashboard
bot
added
the
tool
jonahwilliams
changed the title
| @@ -445,6 +447,152 @@ class FakePub extends Fake implements Pub { | |||
| }) async { } | |||
| } | |||
|
|
|||
| class FakeFlutterVersion implements FlutterVersion { | |||
There was a problem hiding this comment.
I know you just moved this, but this test class doesn't seem tight enough considering it's not actually a Fake and the FlutterVersion constructor is calling git commands.
There was a problem hiding this comment.
I agree, thankfully its not widely used so we should probably find a better pattern.
There was a problem hiding this comment.
Oh nm it's an implements, does this work if you extends Fake?
There was a problem hiding this comment.
FlutterVersion was one of the most painful to inject into tests because of those git calls.
There was a problem hiding this comment.
What I mean is that I agree we should fix this, but I think since this PR didn't actually modify it (and it is still default injected in the testbed groups) I don't think I should expand the scope of this PR to also fix it.
There was a problem hiding this comment.
I just checked, the tests pass with:
class TestFeatureFlags extends Fake implements FeatureFlags {class FakeFlutterVersion extends Fake implements FlutterVersion {I know you just moved this, but
I think since this PR didn't actually modify it I don't think I should expand the scope of this PR to also fix it.
Fair enough. I can play with it if I adopt FakeFlutterVersion in more places.
There was a problem hiding this comment.
TestFeatureFlags definitely isn't a fake though, its pretty specifically designed
To reduce the number of fakes used for classes where we've already created a test implementation. Also move the fakes from the testbed lib into fakes.dart