[flutter_plugin_tests] Split analyze out of xctest #4161

stuartmorgan · 2021-07-16T15:15:51Z

To prep for making a combined command to run native tests across different platforms, rework xctest:

Split analyze out into a new xcode-analyze command:
- Since the analyze step runs a new build over everything with different flags, this is only a small amount slower than the combined version
- This makes the logic easier to follow
- This allows us to meaningfully report skips, to better notice missing tests.
Add the ability to target specific test bundles (RunnerTests or RunnerUITests)

To share code between the commands, this extracts a new Xcode helper class.

Part of flutter/flutter#84392 and flutter/flutter#86489

Pre-launch Checklist

stuartmorgan · 2021-07-16T15:19:21Z

script/tool/test/xctest_command_test.dart

-      'version': '13.0',
-      'isAvailable': true,
-      'name': 'iOS 13.0'
-    },


This testing of finding the best device among many is now part of xcode_test.dart, so what's left here is just one valid device for the test to find.

stuartmorgan · 2021-07-16T15:20:14Z

script/tool/test/xctest_command_test.dart

    group('iOS', () {
      test('skip if iOS is not supported', () async {
-        createFakePlugin('plugin', packagesDir, extraFiles: <String>[
-          'example/test',


Opportunistic cleanup; this extraFiles was copy/pasted from some initial test into lots of other tests that don't actually need it, and I've been cleaning it up as I notice.

stuartmorgan · 2021-07-16T15:21:09Z

script/tool/test/xctest_command_test.dart

          kPlatformIos: PlatformSupport.inline
        });

        final Directory pluginExampleDirectory =
            pluginDirectory.childDirectory('example');

-        processRunner.processToReturn = MockProcess.succeeding();
-        processRunner.resultStdout =
-            '{"project":{"targets":["bar_scheme", "foo_scheme"]}}';


Opportunistic cleanup; I'm assuming that this is from an earlier version of xctest_command, because nothing currently uses this output.

stuartmorgan · 2021-07-16T15:21:56Z

script/tool/test/xctest_command_test.dart


-        final List<String> output = await runCapturingPrint(runner,
-            <String>['xctest', '--macos', _kDestination, 'foo_destination']);


Opportunistic cleanup; I accidentally left the targets when initially copy/pasting iOS tests for macOS.

stuartmorgan · 2021-07-16T15:23:02Z

script/tool/lib/src/xctest_command.dart

@@ -56,9 +59,6 @@ class XCTestCommand extends PackageLoopingCommand {
      'Runs the xctests in the iOS and/or macOS example apps.\n\n'
      'This command requires "flutter" and "xcrun" to be in your path.';

-  @override
-  String get failureListHeader => 'The following packages are failing XCTests:';


Opportunistic cleanup; I left this when doing the initial migration to the new base command to minimize test diff, but there's no reason we need a custom summary header for this command.

…st-target

jmagman · 2021-07-20T21:23:45Z

Add the ability to target specific test bundles (RunnerTests or RunnerUITests)

Both types of tests will run on the same build artifacts, so it should be faster to run them at the same time. It's a fine feature to add, just keep it in mind if you intend to split them into separate CI tests.

jmagman · 2021-07-20T21:13:26Z

script/tool/lib/src/common/xcode.dart

+    final List<String> findSimulatorsArguments = <String>[
+      'simctl',
+      'list',
+      '--json'
+    ];


I know you just copied this, but if you add devices available

Suggested change

final List<String> findSimulatorsArguments = <String>[

'simctl',

'list',

'--json'

];

final List<String> findSimulatorsArguments = <String>[

'simctl',

'list',

'devices',

'available',

'--json'

];

The availabilityError/isAvailable check below can be removed.

Thanks, done! (I had to add 'runtimes' as well since it's parsing those too.)

jmagman · 2021-07-20T21:21:55Z

script/tool/lib/src/xcode_analyze_command.dart

+        ])) {
+      failures.add('iOS');
+    }
+    if (testMacos && !await _analyzePlugin(package, 'macOS')) {


You don't need to worry about this now, but when CI starts using Xcode 13 next year we may need a macOS -destination option.
flutter/flutter#86590

stuartmorgan · 2021-07-20T23:45:27Z

Both types of tests will run on the same build artifacts, so it should be faster to run them at the same time. It's a fine feature to add, just keep it in mind if you intend to split them into separate CI tests.

Yes, I had tested that at the same time I tested splitting out analyze and as expected it is much slower. This is intended for local use (and is prep for the new combined PR that will have options to select unit or integration tests for local use, and maybe for use in CI on platforms where combining them doesn't help).

…lugins#4161)

To prep for making a combined command to run native tests across different platforms, rework `xctest`: - Split analyze out into a new `xcode-analyze` command: - Since the analyze step runs a new build over everything with different flags, this is only a small amount slower than the combined version - This makes the logic easier to follow - This allows us to meaningfully report skips, to better notice missing tests. - Add the ability to target specific test bundles (RunnerTests or RunnerUITests) To share code between the commands, this extracts a new `Xcode` helper class. Part of flutter/flutter#84392 and flutter/flutter#86489

stuartmorgan added 6 commits July 15, 2021 13:19

Add more options to control what runs

3611f35

Report skip, not fail, when analyze is off and there are no tests

47e9d90

Extract a helper for Xcode commands, and add tests for it

33940f5

Migrate xctest to the Xcode class

aba3199

Split out xcode-analyze

fd1958a

Comment fix

b27265a

google-cla bot added the cla: yes label Jul 16, 2021

Remove unnecessary process mocking

1a357e5

stuartmorgan commented Jul 16, 2021

View reviewed changes

Skip, rather than failing, when a package doesn't have a requested te…

02945c9

…st-target

stuartmorgan requested a review from jmagman July 20, 2021 20:11

jmagman approved these changes Jul 20, 2021

View reviewed changes

stuartmorgan added 2 commits July 20, 2021 19:48

Merge branch 'master' into xctest-refactor

b4be22c

Filter out unavailable via simctl

be1596a

stuartmorgan force-pushed the xctest-refactor branch from d909b95 to be1596a Compare July 20, 2021 23:59

jmagman approved these changes Jul 21, 2021

View reviewed changes

stuartmorgan merged commit 09ec519 into flutter:master Jul 21, 2021

stuartmorgan deleted the xctest-refactor branch July 21, 2021 01:17

engine-flutter-autoroll mentioned this pull request Jul 27, 2021

Roll Plugins from 027db9039c03 to 530a18705f74 (21 revisions) flutter/flutter#87080

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 27, 2021

09ec519 [flutter_plugin_tests] Split analyze out of xctest (flutter/p…

4e22c6a

…lugins#4161)

engine-flutter-autoroll mentioned this pull request Jul 27, 2021

Roll Plugins from 027db9039c03 to 5f42c6920cc6 (22 revisions) flutter/flutter#87088

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 27, 2021

09ec519 [flutter_plugin_tests] Split analyze out of xctest (flutter/p…

6d21dff

…lugins#4161)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[flutter_plugin_tests] Split analyze out of xctest #4161

[flutter_plugin_tests] Split analyze out of xctest #4161

stuartmorgan commented Jul 16, 2021

stuartmorgan Jul 16, 2021

stuartmorgan Jul 16, 2021

stuartmorgan Jul 16, 2021

stuartmorgan Jul 16, 2021

stuartmorgan Jul 16, 2021

jmagman commented Jul 20, 2021

jmagman Jul 20, 2021

stuartmorgan Jul 20, 2021

jmagman Jul 20, 2021

stuartmorgan commented Jul 20, 2021


		final List<String> output = await runCapturingPrint(runner,
		<String>['xctest', '--macos', _kDestination, 'foo_destination']);

[flutter_plugin_tests] Split analyze out of xctest #4161

[flutter_plugin_tests] Split analyze out of xctest #4161

Conversation

stuartmorgan commented Jul 16, 2021

Pre-launch Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmagman commented Jul 20, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuartmorgan commented Jul 20, 2021