TestCompiler emits why an error occurred, if applicable, and some refactors to do so (#160984)

Closes https://github.com/flutter/flutter/issues/160218. Basically, replaces `String?` with `sealed class TestCompilerResult {}`, and ensures `errorMessage` is propogated. We'll be using this path now for _all_ integration tests (not just for web-specific things), so I'd like to get error messages.
2025-01-06 16:20:50 -08:00 · 2025-01-06 16:20:50 -08:00 · 16b9fe049d
commit 16b9fe049d
parent 4f35112363
6 changed files with 189 additions and 72 deletions
--- a/packages/flutter_tools/lib/src/test/flutter_platform.dart
+++ b/packages/flutter_tools/lib/src/test/flutter_platform.dart
@ -656,11 +656,14 @@ class FlutterPlatform extends PlatformPlugin {
            flutterProject,
            testTimeRecorder: testTimeRecorder,
          );
-          mainDart = await compiler!.compile(globals.fs.file(mainDart).uri);
-
-          if (mainDart == null) {
-            testHarnessChannel.sink.addError('Compilation failed for testPath=$testPath');
-            return null;
+          switch (await compiler!.compile(globals.fs.file(mainDart).uri)) {
+            case TestCompilerComplete(:final String outputPath):
+              mainDart = outputPath;
+            case TestCompilerFailure(:final String? error):
+              testHarnessChannel.sink.addError(
+                'Compilation failed for testPath=$testPath: $error.',
+              );
+              return null;
          }
        } else {
          // For integration tests, we may still need to set up expression compilation service.
--- a/packages/flutter_tools/lib/src/test/test_compiler.dart
+++ b/packages/flutter_tools/lib/src/test/test_compiler.dart
@ -17,11 +17,72 @@ import '../project.dart';
 import 'test_time_recorder.dart';

 /// A request to the [TestCompiler] for recompilation.
-class CompilationRequest {
-  CompilationRequest(this.mainUri, this.result);
+final class _CompilationRequest {
+  _CompilationRequest(this.mainUri);

-  Uri mainUri;
-  Completer<String?> result;
+  /// The entrypoint (containing `main()`) to the Dart program being compiled.
+  final Uri mainUri;
+
+  /// Invoked when compilation is completed with the compilation output path.
+  Future<TestCompilerResult> get result => _result.future;
+  final Completer<TestCompilerResult> _result = Completer<TestCompilerResult>();
+}
+
+/// The result of [TestCompiler.compile].
+@immutable
+sealed class TestCompilerResult {
+  const TestCompilerResult({required this.mainUri});
+
+  /// The program that was or was attempted to be compiled.
+  final Uri mainUri;
+}
+
+/// A successful run of [TestCompiler.compile].
+final class TestCompilerComplete extends TestCompilerResult {
+  const TestCompilerComplete({required this.outputPath, required super.mainUri});
+
+  /// Output path of the compiled program.
+  final String outputPath;
+
+  @override
+  bool operator ==(Object other) {
+    if (other is! TestCompilerComplete) {
+      return false;
+    }
+    return mainUri == other.mainUri && outputPath == other.outputPath;
+  }
+
+  @override
+  int get hashCode => Object.hash(mainUri, outputPath);
+
+  @override
+  String toString() {
+    return 'TestCompilerComplete(mainUri: $mainUri, outputPath: $outputPath)';
+  }
+}
+
+/// A failed run of [TestCompiler.compile].
+final class TestCompilerFailure extends TestCompilerResult {
+  const TestCompilerFailure({required this.error, required super.mainUri});
+
+  /// Error message that occurred failing compilation.
+  final String error;
+
+  @override
+  bool operator ==(Object other) {
+    if (other is! TestCompilerFailure) {
+      return false;
+    }
+    return mainUri == other.mainUri && error == other.error;
+  }
+
+  @override
+  int get hashCode => Object.hash(mainUri, error);
+
+  @override
+  String toString() {
+    return 'TestCompilerComplete(mainUri: $mainUri, error: $error)';
+  }
 }

 /// A frontend_server wrapper for the flutter test runner.
@ -79,9 +140,9 @@ class TestCompiler {
    );
  }

-  final StreamController<CompilationRequest> compilerController =
-      StreamController<CompilationRequest>();
-  final List<CompilationRequest> compilationQueue = <CompilationRequest>[];
+  final StreamController<_CompilationRequest> compilerController =
+      StreamController<_CompilationRequest>();
+  final List<_CompilationRequest> compilationQueue = <_CompilationRequest>[];
  final FlutterProject? flutterProject;
  final BuildInfo buildInfo;
  final String testFilePath;
@ -91,13 +152,14 @@ class TestCompiler {
  ResidentCompiler? compiler;
  late File outputDill;

-  Future<String?> compile(Uri mainDart) {
-    final Completer<String?> completer = Completer<String?>();
+  /// Compiles the Dart program (an entrypoint containing `main()`).
+  Future<TestCompilerResult> compile(Uri dartEntrypointPath) {
    if (compilerController.isClosed) {
-      return Future<String?>.value();
+      throw StateError('TestCompiler is already disposed.');
    }
-    compilerController.add(CompilationRequest(mainDart, completer));
-    return completer.future;
+    final _CompilationRequest request = _CompilationRequest(dartEntrypointPath);
+    compilerController.add(request);
+    return request.result;
  }

  Future<void> _shutdown() async {
@ -139,7 +201,7 @@ class TestCompiler {
  }

  // Handle a compilation request.
-  Future<void> _onCompilationRequest(CompilationRequest request) async {
+  Future<void> _onCompilationRequest(_CompilationRequest request) async {
    final bool isEmpty = compilationQueue.isEmpty;
    compilationQueue.add(request);
    // Only trigger processing if queue was empty - i.e. no other requests
@ -149,7 +211,7 @@ class TestCompiler {
      return;
    }
    while (compilationQueue.isNotEmpty) {
-      final CompilationRequest request = compilationQueue.first;
+      final _CompilationRequest request = compilationQueue.first;
      globals.printTrace('Compiling ${request.mainUri}');
      final Stopwatch compilerTime = Stopwatch()..start();
      final Stopwatch? testTimeRecorderStopwatch = testTimeRecorder?.start(TestTimePhases.Compile);
@ -190,7 +252,12 @@ class TestCompiler {
      // compiler to avoid reusing compiler that might have gotten into
      // a weird state.
      if (outputPath == null || compilerOutput!.errorCount > 0) {
-        request.result.complete();
+        request._result.complete(
+          TestCompilerFailure(
+            error: compilerOutput!.errorMessage ?? 'Unknown Error',
+            mainUri: request.mainUri,
+          ),
+        );
        await _shutdown();
      } else {
        if (shouldCopyDillFile) {
@ -209,9 +276,13 @@ class TestCompiler {
            }
            await outputFile.copy(testFilePath);
          }
-          request.result.complete(kernelReadyToRun.path);
+          request._result.complete(
+            TestCompilerComplete(outputPath: kernelReadyToRun.path, mainUri: request.mainUri),
+          );
        } else {
-          request.result.complete(outputPath);
+          request._result.complete(
+            TestCompilerComplete(outputPath: outputPath, mainUri: request.mainUri),
+          );
        }
        compiler!.accept();
        compiler!.reset();
--- a/packages/flutter_tools/lib/src/test/test_golden_comparator.dart
+++ b/packages/flutter_tools/lib/src/test/test_golden_comparator.dart
@ -104,18 +104,21 @@ final class TestGoldenComparator {
    final File listenerFile = (await _tempDir.createTemp('listener')).childFile('listener.dart');
    await listenerFile.writeAsString(testBootstrap);

-    final String? output = await _compiler.compile(listenerFile.uri);
-    if (output == null) {
-      return null;
-    }
-    final List<String> command = <String>[
-      _flutterTesterBinPath,
-      '--disable-vm-service',
-      '--non-interactive',
-      output,
-    ];
+    final TestCompilerResult result = await _compiler.compile(listenerFile.uri);
+    switch (result) {
+      case TestCompilerFailure(:final String error):
+        _logger.printWarning('An error occurred compiling ${listenerFile.uri}: $error.');
+        return null;
+      case TestCompilerComplete(:final String outputPath):
+        final List<String> command = <String>[
+          _flutterTesterBinPath,
+          '--disable-vm-service',
+          '--non-interactive',
+          outputPath,
+        ];

-    return _processManager.start(command, environment: _environment);
+        return _processManager.start(command, environment: _environment);
+    }
  }

  /// Compares the golden file designated by [goldenKey], relative to [testUri], to the provide [bytes].
--- a/packages/flutter_tools/test/general.shard/flutter_platform_test.dart
+++ b/packages/flutter_tools/test/general.shard/flutter_platform_test.dart
@ -330,7 +330,7 @@ void main() {
            '--use-test-fonts',
            '--disable-asset-fonts',
            '--packages=.dart_tool/package_config.json',
-            '',
+            'path_to_output.dill',
          ],
          exitCode: -9,
          completer: testCompleter,
@ -396,7 +396,12 @@ void main() {
      () async {
        processManager.addCommand(
          const FakeCommand(
-            command: <String>['flutter_tester', '--disable-vm-service', '--non-interactive', ''],
+            command: <String>[
+              'flutter_tester',
+              '--disable-vm-service',
+              '--non-interactive',
+              'path_to_output.dill',
+            ],
            stdout: '{"success": true}\n',
          ),
        );
@ -524,7 +529,9 @@ class _FakeVmService extends Fake implements VmService {

 class _FakeTestCompiler extends Fake implements TestCompiler {
  @override
-  Future<String?> compile(Uri mainDart) async => '';
+  Future<TestCompilerResult> compile(Uri mainUri) async {
+    return TestCompilerComplete(outputPath: 'path_to_output.dill', mainUri: mainUri);
+  }
 }

 class _UnstartableDevice extends Fake implements Device {
--- a/packages/flutter_tools/test/general.shard/test/test_compiler_test.dart
+++ b/packages/flutter_tools/test/general.shard/test/test_compiler_test.dart
@ -63,7 +63,11 @@ void main() {
        residentCompiler,
      );

-      expect(await testCompiler.compile(Uri.parse('test/foo.dart')), 'test/foo.dart.dill');
+      final Uri input = Uri.parse('test/foo.dart');
+      expect(
+        await testCompiler.compile(input),
+        TestCompilerComplete(outputPath: 'test/foo.dart.dill', mainUri: input),
+      );
    },
    overrides: <Type, Generator>{
      FileSystem: () => fileSystem,
@ -86,7 +90,11 @@ void main() {
        precompiledDillPath: 'precompiled.dill',
      );

-      expect(await testCompiler.compile(Uri.parse('test/foo.dart')), 'abc.dill');
+      final Uri input = Uri.parse('test/foo.dart');
+      expect(
+        await testCompiler.compile(input),
+        TestCompilerComplete(outputPath: 'abc.dill', mainUri: input),
+      );
    },
    overrides: <Type, Generator>{
      FileSystem: () => fileSystem,
@ -99,16 +107,25 @@ void main() {
  );

  testUsingContext(
-    'TestCompiler reports null when a compile fails',
+    'TestCompiler reports an error when a compile fails',
    () async {
-      residentCompiler.compilerOutput = const CompilerOutput('abc.dill', 1, <Uri>[]);
+      residentCompiler.compilerOutput = const CompilerOutput(
+        'abc.dill',
+        1,
+        <Uri>[],
+        errorMessage: 'A big bad happened',
+      );
      final FakeTestCompiler testCompiler = FakeTestCompiler(
        debugBuild,
        FlutterProject.fromDirectoryTest(fileSystem.currentDirectory),
        residentCompiler,
      );

-      expect(await testCompiler.compile(Uri.parse('test/foo.dart')), null);
+      final Uri input = Uri.parse('test/foo.dart');
+      expect(
+        await testCompiler.compile(input),
+        TestCompilerFailure(error: 'A big bad happened', mainUri: input),
+      );
      expect(residentCompiler.didShutdown, true);
    },
    overrides: <Type, Generator>{
@ -132,7 +149,12 @@ void main() {
        residentCompiler,
        testTimeRecorder: testTimeRecorder,
      );
-      expect(await testCompiler.compile(Uri.parse('test/foo.dart')), 'test/foo.dart.dill');
+
+      final Uri input = Uri.parse('test/foo.dart');
+      expect(
+        await testCompiler.compile(Uri.parse('test/foo.dart')),
+        TestCompilerComplete(outputPath: 'test/foo.dart.dill', mainUri: input),
+      );
      testTimeRecorder.print();

      // Expect one message for each phase.
--- a/packages/flutter_tools/test/general.shard/test/test_golden_comparator_test.dart
+++ b/packages/flutter_tools/test/general.shard/test/test_golden_comparator_test.dart
@ -31,12 +31,32 @@ void main() {
    logger = BufferLogger.test();
  });

+  FakeCommand fakeFluterTester(
+    String pathToBinTool, {
+    required String stdout,
+    required Uri mainUri,
+    Map<String, String>? environment,
+    Completer<void>? waitUntil,
+  }) {
+    return FakeCommand(
+      command: <String>[
+        pathToBinTool,
+        '--disable-vm-service',
+        '--non-interactive',
+        'path_to_compiler_output.dill',
+      ],
+      stdout: stdout,
+      environment: environment,
+      completer: waitUntil,
+    );
+  }
+
  testWithoutContext('should succeed when a golden-file comparison matched', () async {
    final TestGoldenComparator comparator = TestGoldenComparator(
      compilerFactory: _FakeTestCompiler.new,
      flutterTesterBinPath: 'flutter_tester',
      processManager: FakeProcessManager.list(<FakeCommand>[
-        _fakeFluterTester('flutter_tester', stdout: _encodeStdout(success: true)),
+        fakeFluterTester('flutter_tester', stdout: _encodeStdout(success: true), mainUri: testUri1),
      ]),
      fileSystem: fileSystem,
      logger: logger,
@ -51,7 +71,11 @@ void main() {
      compilerFactory: _FakeTestCompiler.new,
      flutterTesterBinPath: 'flutter_tester',
      processManager: FakeProcessManager.list(<FakeCommand>[
-        _fakeFluterTester('flutter_tester', stdout: _encodeStdout(success: false)),
+        fakeFluterTester(
+          'flutter_tester',
+          stdout: _encodeStdout(success: false),
+          mainUri: testUri1,
+        ),
      ]),
      fileSystem: fileSystem,
      logger: logger,
@ -66,9 +90,10 @@ void main() {
      compilerFactory: _FakeTestCompiler.new,
      flutterTesterBinPath: 'flutter_tester',
      processManager: FakeProcessManager.list(<FakeCommand>[
-        _fakeFluterTester(
+        fakeFluterTester(
          'flutter_tester',
          stdout: _encodeStdout(success: false, message: 'Did a bad'),
+          mainUri: testUri1,
        ),
      ]),
      fileSystem: fileSystem,
@ -84,7 +109,7 @@ void main() {
      compilerFactory: _FakeTestCompiler.new,
      flutterTesterBinPath: 'flutter_tester',
      processManager: FakeProcessManager.list(<FakeCommand>[
-        _fakeFluterTester('flutter_tester', stdout: _encodeStdout(success: true)),
+        fakeFluterTester('flutter_tester', stdout: _encodeStdout(success: true), mainUri: testUri1),
      ]),
      fileSystem: fileSystem,
      logger: logger,
@ -99,9 +124,10 @@ void main() {
      compilerFactory: _FakeTestCompiler.new,
      flutterTesterBinPath: 'flutter_tester',
      processManager: FakeProcessManager.list(<FakeCommand>[
-        _fakeFluterTester(
+        fakeFluterTester(
          'flutter_tester',
          stdout: _encodeStdout(success: false, message: 'Did a bad'),
+          mainUri: testUri1,
        ),
      ]),
      fileSystem: fileSystem,
@ -117,10 +143,11 @@ void main() {
      compilerFactory: _FakeTestCompiler.new,
      flutterTesterBinPath: 'flutter_tester',
      processManager: FakeProcessManager.list(<FakeCommand>[
-        _fakeFluterTester(
+        fakeFluterTester(
          'flutter_tester',
          stdout: _encodeStdout(success: true),
          environment: <String, String>{'THE_ANSWER': '42'},
+          mainUri: testUri1,
        ),
      ]),
      fileSystem: fileSystem,
@ -137,12 +164,13 @@ void main() {
      compilerFactory: _FakeTestCompiler.new,
      flutterTesterBinPath: 'flutter_tester',
      processManager: FakeProcessManager.list(<FakeCommand>[
-        _fakeFluterTester(
+        fakeFluterTester(
          'flutter_tester',
          stdout: <String>[
            _encodeStdout(success: false, message: '1 Did a bad'),
            _encodeStdout(success: false, message: '2 Did a bad'),
          ].join('\n'),
+          mainUri: testUri1,
        ),
      ]),
      fileSystem: fileSystem,
@ -161,13 +189,15 @@ void main() {
      compilerFactory: _FakeTestCompiler.new,
      flutterTesterBinPath: 'flutter_tester',
      processManager: FakeProcessManager.list(<FakeCommand>[
-        _fakeFluterTester(
+        fakeFluterTester(
          'flutter_tester',
          stdout: _encodeStdout(success: false, message: '1 Did a bad'),
+          mainUri: testUri1,
        ),
-        _fakeFluterTester(
+        fakeFluterTester(
          'flutter_tester',
          stdout: _encodeStdout(success: false, message: '2 Did a bad'),
+          mainUri: testUri2,
        ),
      ]),
      fileSystem: fileSystem,
@ -211,25 +241,6 @@ void main() {
  });
 }

-FakeCommand _fakeFluterTester(
-  String pathToBinTool, {
-  required String stdout,
-  Map<String, String>? environment,
-  Completer<void>? waitUntil,
-}) {
-  return FakeCommand(
-    command: <String>[
-      pathToBinTool,
-      '--disable-vm-service',
-      '--non-interactive',
-      'compiler_output',
-    ],
-    stdout: stdout,
-    environment: environment,
-    completer: waitUntil,
-  );
-}
-
 String _encodeStdout({required bool success, String? message}) {
  return jsonEncode(<String, Object?>{'success': success, if (message != null) 'message': message});
 }
@ -238,8 +249,8 @@ final class _FakeTestCompiler extends Fake implements TestCompiler {
  bool disposed = false;

  @override
-  Future<String> compile(Uri mainDart) {
-    return Future<String>.value('compiler_output');
+  Future<TestCompilerResult> compile(Uri mainDart) async {
+    return TestCompilerComplete(outputPath: 'path_to_compiler_output.dill', mainUri: mainDart);
  }

  @override