Skip to content

Commit 16b9fe0

Browse files
matanlureymatanlurey
authored
TestCompiler emits why an error occurred, if applicable, and some refactors to do so (#160984)
Closes flutter/flutter#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.
1 parent 4f35112 commit 16b9fe0

File tree

6 files changed

+189
-72
lines changed

6 files changed

+189
-72
lines changed

‎packages/flutter_tools/lib/src/test/flutter_platform.dart‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -656,11 +656,14 @@ class FlutterPlatform extends PlatformPlugin {
656656
flutterProject,
657657
testTimeRecorder: testTimeRecorder,
658658
);
659-
mainDart = await compiler!.compile(globals.fs.file(mainDart).uri);
660-
661-
if (mainDart == null) {
662-
testHarnessChannel.sink.addError('Compilation failed for testPath=$testPath');
663-
return null;
659+
switch (await compiler!.compile(globals.fs.file(mainDart).uri)) {
660+
case TestCompilerComplete(:final String outputPath):
661+
mainDart = outputPath;
662+
case TestCompilerFailure(:final String? error):
663+
testHarnessChannel.sink.addError(
664+
'Compilation failed for testPath=$testPath: $error.',
665+
);
666+
return null;
664667
}
665668
} else {
666669
// For integration tests, we may still need to set up expression compilation service.

‎packages/flutter_tools/lib/src/test/test_compiler.dart‎

Lines changed: 88 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,72 @@ import '../project.dart';
1717
import 'test_time_recorder.dart';
1818

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

23-
Uri mainUri;
24-
Completer<String?> result;
23+
/// The entrypoint (containing `main()`) to the Dart program being compiled.
24+
final Uri mainUri;
25+
26+
/// Invoked when compilation is completed with the compilation output path.
27+
Future<TestCompilerResult> get result => _result.future;
28+
final Completer<TestCompilerResult> _result = Completer<TestCompilerResult>();
29+
}
30+
31+
/// The result of [TestCompiler.compile].
32+
@immutable
33+
sealed class TestCompilerResult {
34+
const TestCompilerResult({required this.mainUri});
35+
36+
/// The program that was or was attempted to be compiled.
37+
final Uri mainUri;
38+
}
39+
40+
/// A successful run of [TestCompiler.compile].
41+
final class TestCompilerComplete extends TestCompilerResult {
42+
const TestCompilerComplete({required this.outputPath, required super.mainUri});
43+
44+
/// Output path of the compiled program.
45+
final String outputPath;
46+
47+
@override
48+
bool operator ==(Object other) {
49+
if (other is! TestCompilerComplete) {
50+
return false;
51+
}
52+
return mainUri == other.mainUri && outputPath == other.outputPath;
53+
}
54+
55+
@override
56+
int get hashCode => Object.hash(mainUri, outputPath);
57+
58+
@override
59+
String toString() {
60+
return 'TestCompilerComplete(mainUri: $mainUri, outputPath: $outputPath)';
61+
}
62+
}
63+
64+
/// A failed run of [TestCompiler.compile].
65+
final class TestCompilerFailure extends TestCompilerResult {
66+
const TestCompilerFailure({required this.error, required super.mainUri});
67+
68+
/// Error message that occurred failing compilation.
69+
final String error;
70+
71+
@override
72+
bool operator ==(Object other) {
73+
if (other is! TestCompilerFailure) {
74+
return false;
75+
}
76+
return mainUri == other.mainUri && error == other.error;
77+
}
78+
79+
@override
80+
int get hashCode => Object.hash(mainUri, error);
81+
82+
@override
83+
String toString() {
84+
return 'TestCompilerComplete(mainUri: $mainUri, error: $error)';
85+
}
2586
}
2687

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

82-
final StreamController<CompilationRequest> compilerController =
83-
StreamController<CompilationRequest>();
84-
final List<CompilationRequest> compilationQueue = <CompilationRequest>[];
143+
final StreamController<_CompilationRequest> compilerController =
144+
StreamController<_CompilationRequest>();
145+
final List<_CompilationRequest> compilationQueue = <_CompilationRequest>[];
85146
final FlutterProject? flutterProject;
86147
final BuildInfo buildInfo;
87148
final String testFilePath;
@@ -91,13 +152,14 @@ class TestCompiler {
91152
ResidentCompiler? compiler;
92153
late File outputDill;
93154

94-
Future<String?> compile(Uri mainDart) {
95-
final Completer<String?> completer = Completer<String?>();
155+
/// Compiles the Dart program (an entrypoint containing `main()`).
156+
Future<TestCompilerResult> compile(Uri dartEntrypointPath) {
96157
if (compilerController.isClosed) {
97-
return Future<String?>.value();
158+
throw StateError('TestCompiler is already disposed.');
98159
}
99-
compilerController.add(CompilationRequest(mainDart, completer));
100-
return completer.future;
160+
final _CompilationRequest request = _CompilationRequest(dartEntrypointPath);
161+
compilerController.add(request);
162+
return request.result;
101163
}
102164

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

141203
// Handle a compilation request.
142-
Future<void> _onCompilationRequest(CompilationRequest request) async {
204+
Future<void> _onCompilationRequest(_CompilationRequest request) async {
143205
final bool isEmpty = compilationQueue.isEmpty;
144206
compilationQueue.add(request);
145207
// Only trigger processing if queue was empty - i.e. no other requests
@@ -149,7 +211,7 @@ class TestCompiler {
149211
return;
150212
}
151213
while (compilationQueue.isNotEmpty) {
152-
final CompilationRequest request = compilationQueue.first;
214+
final _CompilationRequest request = compilationQueue.first;
153215
globals.printTrace('Compiling ${request.mainUri}');
154216
final Stopwatch compilerTime = Stopwatch()..start();
155217
final Stopwatch? testTimeRecorderStopwatch = testTimeRecorder?.start(TestTimePhases.Compile);
@@ -190,7 +252,12 @@ class TestCompiler {
190252
// compiler to avoid reusing compiler that might have gotten into
191253
// a weird state.
192254
if (outputPath == null || compilerOutput!.errorCount > 0) {
193-
request.result.complete();
255+
request._result.complete(
256+
TestCompilerFailure(
257+
error: compilerOutput!.errorMessage ?? 'Unknown Error',
258+
mainUri: request.mainUri,
259+
),
260+
);
194261
await _shutdown();
195262
} else {
196263
if (shouldCopyDillFile) {
@@ -209,9 +276,13 @@ class TestCompiler {
209276
}
210277
await outputFile.copy(testFilePath);
211278
}
212-
request.result.complete(kernelReadyToRun.path);
279+
request._result.complete(
280+
TestCompilerComplete(outputPath: kernelReadyToRun.path, mainUri: request.mainUri),
281+
);
213282
} else {
214-
request.result.complete(outputPath);
283+
request._result.complete(
284+
TestCompilerComplete(outputPath: outputPath, mainUri: request.mainUri),
285+
);
215286
}
216287
compiler!.accept();
217288
compiler!.reset();

‎packages/flutter_tools/lib/src/test/test_golden_comparator.dart‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -104,18 +104,21 @@ final class TestGoldenComparator {
104104
final File listenerFile = (await _tempDir.createTemp('listener')).childFile('listener.dart');
105105
await listenerFile.writeAsString(testBootstrap);
106106

107-
final String? output = await _compiler.compile(listenerFile.uri);
108-
if (output == null) {
109-
return null;
107+
final TestCompilerResult result = await _compiler.compile(listenerFile.uri);
108+
switch (result) {
109+
case TestCompilerFailure(:final String error):
110+
_logger.printWarning('An error occurred compiling ${listenerFile.uri}: $error.');
111+
return null;
112+
case TestCompilerComplete(:final String outputPath):
113+
final List<String> command = <String>[
114+
_flutterTesterBinPath,
115+
'--disable-vm-service',
116+
'--non-interactive',
117+
outputPath,
118+
];
119+
120+
return _processManager.start(command, environment: _environment);
110121
}
111-
final List<String> command = <String>[
112-
_flutterTesterBinPath,
113-
'--disable-vm-service',
114-
'--non-interactive',
115-
output,
116-
];
117-
118-
return _processManager.start(command, environment: _environment);
119122
}
120123

121124
/// Compares the golden file designated by [goldenKey], relative to [testUri], to the provide [bytes].

‎packages/flutter_tools/test/general.shard/flutter_platform_test.dart‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ void main() {
330330
'--use-test-fonts',
331331
'--disable-asset-fonts',
332332
'--packages=.dart_tool/package_config.json',
333-
'',
333+
'path_to_output.dill',
334334
],
335335
exitCode: -9,
336336
completer: testCompleter,
@@ -396,7 +396,12 @@ void main() {
396396
() async {
397397
processManager.addCommand(
398398
const FakeCommand(
399-
command: <String>['flutter_tester', '--disable-vm-service', '--non-interactive', ''],
399+
command: <String>[
400+
'flutter_tester',
401+
'--disable-vm-service',
402+
'--non-interactive',
403+
'path_to_output.dill',
404+
],
400405
stdout: '{"success": true}\n',
401406
),
402407
);
@@ -524,7 +529,9 @@ class _FakeVmService extends Fake implements VmService {
524529

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

530537
class _UnstartableDevice extends Fake implements Device {

‎packages/flutter_tools/test/general.shard/test/test_compiler_test.dart‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,11 @@ void main() {
6363
residentCompiler,
6464
);
6565

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

89-
expect(await testCompiler.compile(Uri.parse('test/foo.dart')), 'abc.dill');
93+
final Uri input = Uri.parse('test/foo.dart');
94+
expect(
95+
await testCompiler.compile(input),
96+
TestCompilerComplete(outputPath: 'abc.dill', mainUri: input),
97+
);
9098
},
9199
overrides: <Type, Generator>{
92100
FileSystem: () => fileSystem,
@@ -99,16 +107,25 @@ void main() {
99107
);
100108

101109
testUsingContext(
102-
'TestCompiler reports null when a compile fails',
110+
'TestCompiler reports an error when a compile fails',
103111
() async {
104-
residentCompiler.compilerOutput = const CompilerOutput('abc.dill', 1, <Uri>[]);
112+
residentCompiler.compilerOutput = const CompilerOutput(
113+
'abc.dill',
114+
1,
115+
<Uri>[],
116+
errorMessage: 'A big bad happened',
117+
);
105118
final FakeTestCompiler testCompiler = FakeTestCompiler(
106119
debugBuild,
107120
FlutterProject.fromDirectoryTest(fileSystem.currentDirectory),
108121
residentCompiler,
109122
);
110123

111-
expect(await testCompiler.compile(Uri.parse('test/foo.dart')), null);
124+
final Uri input = Uri.parse('test/foo.dart');
125+
expect(
126+
await testCompiler.compile(input),
127+
TestCompilerFailure(error: 'A big bad happened', mainUri: input),
128+
);
112129
expect(residentCompiler.didShutdown, true);
113130
},
114131
overrides: <Type, Generator>{
@@ -132,7 +149,12 @@ void main() {
132149
residentCompiler,
133150
testTimeRecorder: testTimeRecorder,
134151
);
135-
expect(await testCompiler.compile(Uri.parse('test/foo.dart')), 'test/foo.dart.dill');
152+
153+
final Uri input = Uri.parse('test/foo.dart');
154+
expect(
155+
await testCompiler.compile(Uri.parse('test/foo.dart')),
156+
TestCompilerComplete(outputPath: 'test/foo.dart.dill', mainUri: input),
157+
);
136158
testTimeRecorder.print();
137159

138160
// Expect one message for each phase.

0 commit comments

Comments
 (0)