-
Notifications
You must be signed in to change notification settings - Fork 27.2k
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
Swap xcodeproj_tests from MockProcessManager to FakeProcessManager #56505
Conversation
command: <String>[ | ||
xcodebuild, | ||
'-project', | ||
fileSystem.path.separator, |
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.
This was the test that should have failed but didn't.
platform.pathSeparator
is null, fileSystem.path.separator
is /
but this processManager runSync
was never being verified.
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 with nits
This is a great cleanup!
flakes: 1, | ||
delay: delay + const Duration(seconds: 1), | ||
); | ||
platform.environment = Map<String, String>.unmodifiable(<String, String>{}); |
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.
const <String, String>{}
will also create an unmodifiable map
'-project', | ||
'/', | ||
'-target', | ||
'', |
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.
This line is odd ... is this a stray empty string or are we default to ''
for some unset value?
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.
The second parameter in await xcodeProjectInterpreter.getBuildSettings('', '')
is the target, and it's just trusting that input as the target name. I can make it Runner or something more obvious.
Description
Change most of the xcodeproj_tests from MockProcessManager to FakeProcessManager. I had to keep MockProcessManager for the one test that needed to throw on run to work around #56415, and another that was using the
flakyProcessFactory
.Related Issues
I started working on #56489 and none of the tests in this file that should have been failing were doing so. Used the FakeProcessManager to spot the calls that weren't being verified with the right parameters.
Checklist
///
).flutter analyze --flutter-repo
) does not report any problems on my PR.Breaking Change