-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[flutter_plugin_tools] Replace xctest and java-test with native-test #4176
Conversation
It looks like the overall change broke the rename detection threshold, so it'll be easier to review the individual commits rather than the whole PR's diff. Also, if you'd rather I split this into "convert xctest to native-test" and "fold java-test into native-test" as separate PRs I can easily do that. I was going to do it initially, but I thought seeing what it looked like with more than one platform's test might make the overall structure clearer. |
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 w/ spelling nit
extraFlags: _iosDestinationFlags); | ||
} | ||
|
||
Future<_PlatformResult> _testMacOs(Directory plugin, _TestMode mode) { |
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.
Future<_PlatformResult> _testMacOs(Directory plugin, _TestMode mode) { | |
Future<_PlatformResult> _testMacOS(Directory plugin, _TestMode mode) { |
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 never know what to do with iOS and macOS in Dart code :P I hadn't realized there was an exception for two-letter acronyms in Dart style though, thanks for flagging this!
if (mode.unitOnly) { | ||
testTarget = 'RunnerTests'; | ||
} else if (mode.integrationOnly) { | ||
testTarget = 'RunnerUITests'; |
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.
You don't need to address it in this PR since I know this was copied, but I just remembered about the image_picker RunnerUITestiOS14
target. It probably doesn't make sense for that to be a separate target, I wasn't involved in the recent creation of that.
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.
Yes, I found that when checking the repo to make sure this would work, and discussed it with folks this morning; it should get folded back in in the next day or two.
…lutter#4176) Creates a new `native-test` command that will be used to run native unit and UI/integration tests for all platforms over time. This replaces both `xctest` and `java-test`. For CI we can continue to run each platform separately for clarity, but the combined command makes it easier to use (and remember how to use) for local development, as well as avoiding the need to introduce several new commands for desktop testing as support for that is added to the tool. Fixes flutter/flutter#84392 Fixes flutter/flutter#86489
…lutter#4176) Creates a new `native-test` command that will be used to run native unit and UI/integration tests for all platforms over time. This replaces both `xctest` and `java-test`. For CI we can continue to run each platform separately for clarity, but the combined command makes it easier to use (and remember how to use) for local development, as well as avoiding the need to introduce several new commands for desktop testing as support for that is added to the tool. Fixes flutter/flutter#84392 Fixes flutter/flutter#86489
Creates a new
native-test
command that will be used to run native unit and UI/integration tests for all platforms over time. This replaces bothxctest
andjava-test
.For CI we can continue to run each platform separately for clarity, but the combined command makes it easier to use (and remember how to use) for local development, as well as avoiding the need to introduce several new commands for desktop testing as support for that is added to the tool.
Fixes flutter/flutter#84392
Fixes flutter/flutter#86489
Pre-launch Checklist
dart format
.)[shared_preferences]
///
).