[macOS] Refactor the `flutter run` macOS console output test #114645

loic-sharma · 2022-11-03T22:10:51Z

The run_release_test_macos integration test verifies that that the console output of flutter run -d macos --release is expected. This change introduces a RunOutputTask abstraction so that the test's logic can be reused by other platforms like Android and Windows. This new RunOutputTask mirrors other existing device lab tasks like BuildTestTask and PerfTest.

In the future, we will:

Move the existing run_release_test Android integration test to RunOutputTask
Add a new flutter run console output integration test for Windows (see Test that flutter run doesn't introduce new debug spew #111577)
Consider adding flutter run console output tests for other platforms like iOS, Linux, web

Part of #111577

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

loic-sharma · 2022-11-03T22:14:44Z

dev/devicelab/bin/tasks/run_release_test_macos.dart

-          if (
-            !line.startsWith('Building flutter tool...') &&
-            !line.startsWith('Running "flutter pub get" in ui...') &&
-            !line.startsWith('Resolving dependencies...') &&
-            // Catch engine piped output from unrelated concurrent Flutter apps
-            !line.contains(RegExp(r'[A-Z]\/flutter \([0-9]+\):')) &&
-            // Empty lines could be due to the progress spinner breaking up.
-            line.length > 1
-          ) {
-            stdout.add(line);
-          }


FYI I removed this logic to ignore standard output as it had no effect on the test. The _findNextMatcherInList helper used to verify console output can skip lines.

jmagman · 2022-11-04T20:49:27Z

dev/devicelab/lib/tasks/run_tests.dart

+      final String nextLine = list.first;
+      list.removeAt(0);
+
+      if (matcher(nextLine)) {


This may cause out-of-band failures when Xcode is updated, since Xcode loves to add debugging spew, and the macOS build output filtering is very manual:

flutter/packages/flutter_tools/lib/src/macos/build_macos.dart

Lines 22 to 28 in 850f3b3

/// When run in -quiet mode, Xcode should only print from the underlying tasks to stdout.

/// Passing this regexp to trace moves the stdout output to stderr.

///

/// Filter out xcodebuild logging unrelated to macOS builds:

/// xcodebuild[2096:1927385] Requested but did not find extension point with identifier Xcode.IDEKit.ExtensionPointIdentifierToBundleIdentifier for extension Xcode.DebuggerFoundation.AppExtensionToBundleIdentifierMap.watchOS of plug-in com.apple.dt.IDEWatchSupportCore

/// note: Using new build system

final RegExp _filteredOutput = RegExp(r'^((?!Requested but did not find extension point with identifier|note\:).)*$');

(as opposed to iOS where the tool output comes from parsing the Xcode build result, not from Xcode build stdout, would really really love macOS to adopt this)

flutter/packages/flutter_tools/lib/src/ios/mac.dart

Lines 406 to 409 in 8aad119

final XCResultIssueDiscarder warningDiscarder = XCResultIssueDiscarder(typeMatcher: XCResultIssueType.warning);

final XCResultIssueDiscarder dartBuildErrorDiscarder = XCResultIssueDiscarder(messageMatcher: RegExp(r'Command PhaseScriptExecution failed with a nonzero exit code'));

final XCResultGenerator xcResultGenerator = XCResultGenerator(resultPath: resultBundle.absolute.path, xcode: globals.xcode!, processUtils: globals.processUtils);

xcResult = await xcResultGenerator.generate(issueDiscarders: <XCResultIssueDiscarder>[warningDiscarder, dartBuildErrorDiscarder]);

Either we can make this more flexible and accept any upcoming line, or we can accept that additional filters may need to be applied as part of updating Xcode.

I don't think this is a problem as this test ignores unmatched lines. It only checks that the expected lines are in standard output in the correct order. Any Xcode log spew on standard output would be ignored by this test.

Please let me know if I misunderstood your concern!

Please let me know if I misunderstood your concern!

Nah I just have Friday brain, your logic LGTM.

jmagman

LGTM

…lutter/flutter#114645)

* a84e369 Revert "Allow Flutter golden file tests to be flaky (#114450)" (flutter/flutter#114902) * 1725a26 Switch the way we retrieve the vm_service_port from /hub to iquery, on device. (flutter/flutter#114834) * 7020f59 [tool] Support `--flavor` option for `flutter install`. (flutter/flutter#114048) * 9797d5f [macOS] Refactor the `flutter run` macOS console output test (flutter/flutter#114645) * 530324d Build command dependency injection (flutter/flutter#114383) * 69a542d Apply multidex config in kotlin dsl gradle file (flutter/flutter#114660) * 92f10ed match error statements without relying on volatile human-readable descriptions (flutter/flutter#114922) * d3dcd7d Update `CircleAvatar` to support Material 3 (flutter/flutter#114812)

…#114645)

* a84e369 Revert "Allow Flutter golden file tests to be flaky (#114450)" (flutter/flutter#114902) * 1725a26 Switch the way we retrieve the vm_service_port from /hub to iquery, on device. (flutter/flutter#114834) * 7020f59 [tool] Support `--flavor` option for `flutter install`. (flutter/flutter#114048) * 9797d5f [macOS] Refactor the `flutter run` macOS console output test (flutter/flutter#114645) * 530324d Build command dependency injection (flutter/flutter#114383) * 69a542d Apply multidex config in kotlin dsl gradle file (flutter/flutter#114660) * 92f10ed match error statements without relying on volatile human-readable descriptions (flutter/flutter#114922) * d3dcd7d Update `CircleAvatar` to support Material 3 (flutter/flutter#114812)

flutter-dashboard bot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Nov 3, 2022

loic-sharma commented Nov 3, 2022

View reviewed changes

loic-sharma requested review from yaakovschectman, jmagman, cbracken, a-wallen and christopherfujino November 3, 2022 23:55

loic-sharma marked this pull request as ready for review November 3, 2022 23:55

yaakovschectman approved these changes Nov 4, 2022

View reviewed changes

jmagman reviewed Nov 4, 2022

View reviewed changes

jmagman approved these changes Nov 4, 2022

View reviewed changes

[macOS] Refactor the flutter run macOS console output test

b77f6f0

loic-sharma force-pushed the macos_run_test branch from 7fd41c3 to b77f6f0 Compare November 8, 2022 18:47

loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 8, 2022

auto-submit bot merged commit 9797d5f into flutter:master Nov 8, 2022

loic-sharma deleted the macos_run_test branch November 8, 2022 23:10

This was referenced Nov 9, 2022

Roll Flutter from a1432a9c5da2 to d3dcd7d532b6 (8 revisions) flutter/plugins#6684

Merged

Roll Flutter from a1432a9c5da2 to d3dcd7d532b6 (8 revisions) flutter/packages#2791

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Nov 9, 2022

9797d5f [macOS] Refactor the flutter run macOS console output test (f…

7c7c435

…lutter/flutter#114645)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 9, 2022

9797d5f [macOS] Refactor the flutter run macOS console output test (f…

c70c919

…lutter/flutter#114645)

loic-sharma mentioned this pull request Nov 9, 2022

[Android] Refactor the flutter run Android console output test #115023

Merged

8 tasks

loic-sharma mentioned this pull request Nov 21, 2022

[WIP] Test that flutter run doesn't introduce new debug spew #114554

Closed

11 tasks

shogohida pushed a commit to shogohida/flutter that referenced this pull request Dec 7, 2022

[macOS] Refactor the flutter run macOS console output test (flutter…

8d3673c

…#114645)

gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jan 19, 2023

[macOS] Refactor the flutter run macOS console output test (flutter…

0fc6272

…#114645)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[macOS] Refactor the `flutter run` macOS console output test #114645

[macOS] Refactor the `flutter run` macOS console output test #114645

Uh oh!

loic-sharma commented Nov 3, 2022 •

edited

Loading

Uh oh!

loic-sharma Nov 3, 2022 •

edited

Loading

Uh oh!

jmagman Nov 4, 2022

Uh oh!

loic-sharma Nov 4, 2022 •

edited

Loading

Uh oh!

jmagman Nov 4, 2022

Uh oh!

jmagman left a comment

Uh oh!

Uh oh!

	/// When run in -quiet mode, Xcode should only print from the underlying tasks to stdout.
	/// Passing this regexp to trace moves the stdout output to stderr.
	///
	/// Filter out xcodebuild logging unrelated to macOS builds:
	/// xcodebuild[2096:1927385] Requested but did not find extension point with identifier Xcode.IDEKit.ExtensionPointIdentifierToBundleIdentifier for extension Xcode.DebuggerFoundation.AppExtensionToBundleIdentifierMap.watchOS of plug-in com.apple.dt.IDEWatchSupportCore
	/// note: Using new build system
	final RegExp _filteredOutput = RegExp(r'^((?!Requested but did not find extension point with identifier\|note\:).)*$');

	final XCResultIssueDiscarder warningDiscarder = XCResultIssueDiscarder(typeMatcher: XCResultIssueType.warning);
	final XCResultIssueDiscarder dartBuildErrorDiscarder = XCResultIssueDiscarder(messageMatcher: RegExp(r'Command PhaseScriptExecution failed with a nonzero exit code'));
	final XCResultGenerator xcResultGenerator = XCResultGenerator(resultPath: resultBundle.absolute.path, xcode: globals.xcode!, processUtils: globals.processUtils);
	xcResult = await xcResultGenerator.generate(issueDiscarders: <XCResultIssueDiscarder>[warningDiscarder, dartBuildErrorDiscarder]);

[macOS] Refactor the flutter run macOS console output test #114645

[macOS] Refactor the flutter run macOS console output test #114645

Uh oh!

Conversation

loic-sharma commented Nov 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

loic-sharma Nov 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmagman Nov 4, 2022

Choose a reason for hiding this comment

Uh oh!

loic-sharma Nov 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmagman Nov 4, 2022

Choose a reason for hiding this comment

Uh oh!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[macOS] Refactor the `flutter run` macOS console output test #114645

[macOS] Refactor the `flutter run` macOS console output test #114645

loic-sharma commented Nov 3, 2022 •

edited

Loading

loic-sharma Nov 3, 2022 •

edited

Loading

loic-sharma Nov 4, 2022 •

edited

Loading