Remove the need to use runZoned by replacing print statements (flutter#55530)

matanlurey · web-flow · commit a6a7b84ebe25 · 2024-09-30T21:33:21.000Z
diff --git a/tools/engine_tool/lib/src/commands/command.dart b/tools/engine_tool/lib/src/commands/command.dart
@@ -28,6 +28,11 @@ abstract base class CommandBase extends Command<int> {
 
   @override
   final ArgParser argParser;
+
+  @override
+  void printUsage() {
+    environment.logger.status(usage);
+  }
 }
 
 /// Adds the -c (--config) option to the parser.
diff --git a/tools/engine_tool/lib/src/commands/command_runner.dart b/tools/engine_tool/lib/src/commands/command_runner.dart
@@ -103,4 +103,9 @@ final class ToolCommandRunner extends CommandRunner<int> {
       return 1;
     }
   }
+
+  @override
+  void printUsage() {
+    environment.logger.status(usage);
+  }
 }
diff --git a/tools/engine_tool/test/build_command_test.dart b/tools/engine_tool/test/build_command_test.dart
@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:async';
 import 'dart:convert' as convert;
 
 import 'package:engine_build_configs/engine_build_configs.dart';
@@ -449,65 +448,55 @@ void main() {
   });
 
   test('et help build line length is not too big', () async {
-    final prints = <String>[];
-    await runZoned(
-      () async {
-        final testEnv = TestEnvironment.withTestEngine(
-          cannedProcesses: cannedProcesses,
-          verbose: true,
-        );
-        addTearDown(testEnv.cleanup);
-
-        final runner = ToolCommandRunner(
-          environment: testEnv.environment,
-          configs: configs,
-          help: true,
-        );
-        final result = await runner.run([
-          'help',
-          'build',
-        ]);
-        expect(result, equals(0));
-      },
-      zoneSpecification: ZoneSpecification(
-        print: (Zone self, ZoneDelegate parent, Zone zone, String line) {
-          prints.addAll(line.split('\n'));
-        },
-      ),
+    final testEnv = TestEnvironment.withTestEngine(
+      cannedProcesses: cannedProcesses,
+      verbose: true,
+    );
+    addTearDown(testEnv.cleanup);
+
+    final runner = ToolCommandRunner(
+      environment: testEnv.environment,
+      configs: configs,
+      help: true,
+    );
+    final result = await runner.run([
+      'help',
+      'build',
+    ]);
+    expect(result, equals(0));
+
+    // Avoid a degenerate case where nothing is logged.
+    expect(testEnv.testLogs, isNotEmpty, reason: 'No logs were emitted');
+
+    expect(
+      testEnv.testLogs.map((LogRecord r) => r.message.split('\n')),
+      everyElement(hasLength(lessThanOrEqualTo(100))),
     );
-    for (final line in prints) {
-      expect(line.length, lessThanOrEqualTo(100));
-    }
   });
 
   test('non-verbose "et help build" does not contain ci builds', () async {
-    final prints = <String>[];
-    await runZoned(
-      () async {
-        final testEnv = TestEnvironment.withTestEngine(
-          cannedProcesses: cannedProcesses,
-        );
-        addTearDown(testEnv.cleanup);
-
-        final runner = ToolCommandRunner(
-          environment: testEnv.environment,
-          configs: configs,
-          help: true,
-        );
-        final result = await runner.run([
-          'help',
-          'build',
-        ]);
-        expect(result, equals(0));
-      },
-      zoneSpecification: ZoneSpecification(
-        print: (Zone self, ZoneDelegate parent, Zone zone, String line) {
-          prints.addAll(line.split('\n'));
-        },
-      ),
+    final testEnv = TestEnvironment.withTestEngine(
+      cannedProcesses: cannedProcesses,
+    );
+    addTearDown(testEnv.cleanup);
+
+    final runner = ToolCommandRunner(
+      environment: testEnv.environment,
+      configs: configs,
+      help: true,
+    );
+    final result = await runner.run([
+      'help',
+      'build',
+    ]);
+    expect(result, equals(0));
+
+    // Avoid a degenerate case where nothing is logged.
+    expect(testEnv.testLogs, isNotEmpty, reason: 'No logs were emitted');
+
+    expect(
+      testEnv.testLogs.map((LogRecord r) => r.message),
+      isNot(contains('[ci/')),
     );
-    for (final line in prints) {
-      expect(line.contains('[ci/'), isFalse);
-    }
   });
 }

Original file line number	Diff line number	Diff line change
`@@ -103,4 +103,9 @@ final class ToolCommandRunner extends CommandRunner<int> {`
`103`	`103`	`return 1;`
`104`	`104`	`}`
`105`	`105`	`}`
	`106`	`+`
	`107`	`+ @override`
	`108`	`+ void printUsage() {`
	`109`	`+ environment.logger.status(usage);`
	`110`	`+ }`
`106`	`111`	`}`