fix: dont crash on test compilation error (#739)

* fix: dont crash on test compilation error * test everythin * it is justa an and
VeryGoodOpenSource · May 22, 2023 · f19411c · f19411c
1 parent 6b61e9e
commit f19411c
Show file tree

Hide file tree

Showing 10 changed files with 197 additions and 28 deletions.
diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml
@@ -31,6 +31,7 @@ jobs:
         test:
           # E2E tests for the test command
           - test/commands/test/async_main/async_main_test.dart
+          - test/commands/test/compilation_error/compilation_error_test.dart
           - test/commands/test/no_project/no_project_test.dart
           - test/commands/test/spaced_golden_file_name/spaced_golden_file_name_test.dart
 

diff --git a/.github/workflows/very_good_cli.yaml b/.github/workflows/very_good_cli.yaml
@@ -46,6 +46,7 @@ jobs:
 
       - name: Check Code Coverage
         uses: VeryGoodOpenSource/very_good_coverage@v2.1.0
+
   pana:
     runs-on: ubuntu-latest
 

diff --git a/e2e/test/commands/test/compilation_error/compilation_error_test.dart b/e2e/test/commands/test/compilation_error/compilation_error_test.dart
@@ -0,0 +1,42 @@
+import 'package:mason/mason.dart';
+import 'package:mocktail/mocktail.dart';
+import 'package:test/test.dart';
+import 'package:universal_io/io.dart';
+
+import '../../../../helpers/helpers.dart';
+
+void main() {
+  test(
+    'fails when there is a compilation error, but does not crash',
+    timeout: const Timeout(Duration(minutes: 2)),
+    withRunner((commandRunner, logger, updater, logs) async {
+      final tempDirectory =
+          Directory.systemTemp.createTempSync('compilation_error');
+      addTearDown(() => tempDirectory.deleteSync(recursive: true));
+
+      await copyDirectory(
+        Directory('test/commands/test/compilation_error/fixture'),
+        tempDirectory,
+      );
+
+      await expectSuccessfulProcessResult(
+        'flutter',
+        ['pub', 'get'],
+        workingDirectory: tempDirectory.path,
+      );
+
+      final cwd = Directory.current;
+      Directory.current = tempDirectory;
+      addTearDown(() {
+        Directory.current = cwd;
+      });
+
+      final result = await commandRunner.run(['test']);
+
+      expect(result, equals(ExitCode.unavailable.code));
+      verify(
+        () => logger.err(any(that: contains('- test/.test_optimizer.dart'))),
+      ).called(1);
+    }),
+  );
+}
diff --git a/e2e/test/commands/test/compilation_error/fixture/analysis_options.yaml b/e2e/test/commands/test/compilation_error/fixture/analysis_options.yaml
@@ -0,0 +1,3 @@
+analyzer:
+  exclude:
+    - test
diff --git a/e2e/test/commands/test/compilation_error/fixture/lib/compilation_error.dart b/e2e/test/commands/test/compilation_error/fixture/lib/compilation_error.dart
@@ -0,0 +1,3 @@
+class Thing {
+  const Thing();
+}
diff --git a/e2e/test/commands/test/compilation_error/fixture/pubspec.yaml b/e2e/test/commands/test/compilation_error/fixture/pubspec.yaml
@@ -0,0 +1,9 @@
+name: compilation_error
+version: 0.1.0+1
+publish_to: none
+
+environment:
+  sdk: ">=2.19.0 <3.0.0"
+
+dev_dependencies:
+  test: ^1.19.2
diff --git a/e2e/test/commands/test/compilation_error/fixture/test/src/my_package_test.dart b/e2e/test/commands/test/compilation_error/fixture/test/src/my_package_test.dart
@@ -0,0 +1,10 @@
+// ignore_for_file: prefer_const_constructors
+
+import 'package:compilation_error/compilation_error.dart';
+import 'package:test/test.dart';
+
+void main() {
+  test('can be instantiated', () {
+    expect(Thing(thing: true), isNull);
+  });
+}
diff --git a/lib/src/cli/flutter_cli.dart b/lib/src/cli/flutter_cli.dart
@@ -388,16 +388,29 @@ Future<int> _flutterTest({
         if (event.stackTrace.trim().isNotEmpty) {
           stderr('$clearLine${event.stackTrace}');
         }
+
         final test = tests[event.testID]!;
         final suite = suites[test.suiteID]!;
         final prefix = event.isFailure ? '[FAILED]' : '[ERROR]';
 
         final optimizationApplied = _isOptimizationApplied(suite);
-        final topGroupName = _topGroupName(test, groups);
-        final testPath =
-            _actualTestPath(optimizationApplied, suite.path!, topGroupName);
-        final testName =
-            _actualTestName(optimizationApplied, test.name, topGroupName);
+
+        var testPath = suite.path!;
+        var testName = test.name;
+
+        // When there is a test error before any group is computed, it means
+        // that there is an error when compiling the test optimizer file.
+        if (optimizationApplied && groups.isNotEmpty) {
+          final topGroupName = _topGroupName(test, groups)!;
+
+          testPath = testPath.replaceFirst(
+            _testOptimizerFileName,
+            topGroupName,
+          );
+
+          testName = testName.replaceFirst(topGroupName, '').trim();
+        }
+
         final relativeTestPath = p.relative(testPath, from: cwd);
         failedTestErrorMessages[relativeTestPath] = [
           ...failedTestErrorMessages[relativeTestPath] ?? [],
@@ -411,11 +424,18 @@ Future<int> _flutterTest({
         final test = tests[event.testID]!;
         final suite = suites[test.suiteID]!;
         final optimizationApplied = _isOptimizationApplied(suite);
-        final firstGroupName = _topGroupName(test, groups);
-        final testPath =
-            _actualTestPath(optimizationApplied, suite.path!, firstGroupName);
-        final testName =
-            _actualTestName(optimizationApplied, test.name, firstGroupName);
+
+        var testPath = suite.path!;
+        var testName = test.name;
+
+        if (optimizationApplied) {
+          final firstGroupName = _topGroupName(test, groups) ?? '';
+          testPath = testPath.replaceFirst(
+            _testOptimizerFileName,
+            firstGroupName,
+          );
+          testName = testName.replaceFirst(firstGroupName, '').trim();
+        }
 
         if (event.skipped) {
           stdout(
@@ -493,22 +513,6 @@ String? _topGroupName(Test test, Map<int, TestGroup> groups) => test.groupIDs
     .map((groupID) => groups[groupID]?.name)
     .firstWhereOrNull((groupName) => groupName?.isNotEmpty ?? false);
 
-String _actualTestPath(
-  bool optimizationApplied,
-  String path,
-  String? groupName,
-) =>
-    optimizationApplied
-        ? path.replaceFirst(_testOptimizerFileName, groupName!)
-        : path;
-
-String _actualTestName(
-  bool optimizationApplied,
-  String name,
-  String? topGroupName,
-) =>
-    optimizationApplied ? name.replaceFirst(topGroupName!, '').trim() : name;
-
 final int _lineLength = () {
   try {
     return stdout.terminalColumns;
@@ -562,6 +566,7 @@ extension on String {
     return '...$truncated';
   }
 
-  String toSingleLine() =>
-      replaceAll('\n', '').replaceAll(RegExp(r'\s\s+'), ' ');
+  String toSingleLine() {
+    return replaceAll('\n', '').replaceAll(RegExp(r'\s\s+'), ' ');
+  }
 }
diff --git a/test/fixtures/test_runner_fixtures.dart b/test/fixtures/test_runner_fixtures.dart
@@ -1441,3 +1441,54 @@ List<Map<String, Object>> skipExceptionMessageJsonOutput(String cwd) => [
       },
       {'success': false, 'type': 'done', 'time': 5466},
     ];
+
+List<Map<String, Object>> compilationErrorJsonOutput(String cwd) {
+  return [
+    {
+      'protocolVersion': '0.1.1',
+      'runnerVersion': '1.24.3',
+      'pid': 65730,
+      'type': 'start',
+      'time': 0
+    },
+    {
+      'suite': {'id': 0, 'platform': 'vm', 'path': 'test/.test_optimizer.dart'},
+      'type': 'suite',
+      'time': 0
+    },
+    {
+      'test': {
+        'id': 1,
+        'name': 'loading test/.test_optimizer.dart',
+        'suiteID': 0,
+        'groupIDs': <int>[],
+        'metadata': {'skip': false, 'skipReason': null},
+        'line': null,
+        'column': null,
+        'url': null
+      },
+      'type': 'testStart',
+      'time': 0
+    },
+    {'count': 1, 'time': 4, 'type': 'allSuites'},
+    {
+      'testID': 1,
+      'error':
+          "Failed to load \"test/.test_optimizer.dart\":\ntest/src/my_package_test.dart:8:18: Error: No named parameter with the name 'thing'.\n    expect(Thing(thing: true), isNull);\n                 ^^^^^\nlib/compilation_error.dart:2:9: Context: Found this candidate, but the arguments don't match.\n  const Thing();\n        ^^^^^",
+      'stackTrace':
+          'package:test_core/src/runner/vm/platform.dart 255:7   VMPlatform._compileToKernel\n===== asynchronous gap ===========================\npackage:test_core/src/runner/vm/platform.dart 232:15  VMPlatform._spawnIsolate\n===== asynchronous gap ===========================\npackage:test_core/src/runner/vm/platform.dart 76:19   VMPlatform.load\n===== asynchronous gap ===========================\npackage:test_core/src/runner/loader.dart 232:27       Loader.loadFile.<fn>\n===== asynchronous gap ===========================\npackage:test_core/src/runner/load_suite.dart 98:19    new LoadSuite.<fn>.<fn>\n',
+      'isFailure': false,
+      'type': 'error',
+      'time': 532
+    },
+    {
+      'testID': 1,
+      'result': 'error',
+      'skipped': false,
+      'hidden': false,
+      'type': 'testDone',
+      'time': 535
+    },
+    {'success': false, 'type': 'done', 'time': 536},
+  ];
+}
diff --git a/test/src/cli/flutter_cli_test.dart b/test/src/cli/flutter_cli_test.dart
@@ -734,6 +734,50 @@ void main() {
         );
       });
 
+      test('runs tests (compilation error)', () async {
+        final tempDirectory = Directory.systemTemp.createTempSync();
+        addTearDown(() => tempDirectory.deleteSync(recursive: true));
+        Directory(p.join(tempDirectory.path, 'test')).createSync();
+        File(p.join(tempDirectory.path, 'pubspec.yaml')).createSync();
+
+        final testEventStream = Stream.fromIterable([
+          ...compilationErrorJsonOutput(tempDirectory.path)
+              .map(TestEvent.fromJson),
+          const ExitTestEvent(exitCode: 1, time: 0),
+        ]);
+
+        await expectLater(
+          Flutter.test(
+            cwd: tempDirectory.path,
+            stdout: stdoutLogs.add,
+            stderr: stderrLogs.add,
+            testRunner: testRunner(testEventStream),
+            logger: logger,
+          ),
+          completion(equals([ExitCode.unavailable.code])),
+        );
+
+        expect(
+          stdoutLogs,
+          containsAllInOrder([
+            '\x1B[2K\r00:00 -1: loading test/.test_optimizer.dart',
+            '\x1B[2K\r00:00 -1: Some tests failed.\n'
+          ]),
+        );
+        expect(
+          stderrLogs,
+          containsAll([
+            '\x1B[2K\rFailed to load "test/.test_optimizer.dart":\n'
+                "test/src/my_package_test.dart:8:18: Error: No named parameter with the name 'thing'.\n"
+                '    expect(Thing(thing: true), isNull);\n'
+                '                 ^^^^^\n'
+                "lib/compilation_error.dart:2:9: Context: Found this candidate, but the arguments don't match.\n"
+                '  const Thing();\n'
+                '        ^^^^^',
+          ]),
+        );
+      });
+
       test('runs tests w/out logs', () async {
         final tempDirectory = Directory.systemTemp.createTempSync();
         addTearDown(() => tempDirectory.deleteSync(recursive: true));