-
Notifications
You must be signed in to change notification settings - Fork 27.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[flutter_tools] re-use findProjectRoot on flutter command #104850
[flutter_tools] re-use findProjectRoot on flutter command #104850
Conversation
@@ -123,3 +150,30 @@ class FakePub extends Fake implements Pub { | |||
..writeAsStringSync('{"configVersion":2,"packages":[]}'); | |||
} | |||
} | |||
|
|||
class FakePubTargetDirectory extends Fake implements Pub { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about adding an optional parameter to the FakePub
constructor for dartToolDir
that defaults to fileSystem.currentDirectory.childDirectory('.dart_tool')
, rather than creating a whole new class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait, actually, doesn't this already get piped through as the directory
parameter to .get()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait, actually, doesn't this already get piped through as the
directory
parameter to.get()
?
I do not follow. Do you mean that fileSystem.currentDirectory... is not needed? This syncs are needed for the currentDirectory to be a "valid flutter project"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh I see what you mean yes I can just use the directory instead which is already in the function. Good call
|
||
await commandRunner.run(<String>['get', targetDirectory.path]); | ||
|
||
expect(await command.usageValues, const CustomDimensions( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check that .dart_tool/package_config.json
got written?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can delete this expectation actually, as its redundant with the previous test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Fixes: #104760
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.