[coverage] Fix remaining ~0.1% flakiness (#2102)

Author: liamappelbe

pkgs/coverage/CHANGELOG.md

@@ -1,6 +1,9 @@
-## 1.14.1-wip
+## 1.14.1
 
 - Remove dependency on `package:pubspec_parse`.
+- Silence a rare error that can occur when trying to resume the main isolate
+  because the VM service has already shut down. This was responsible for a ~0.1%
+  flakiness, and is safe to ignore.
 
 ## 1.14.0
 

pkgs/coverage/lib/src/isolate_paused_listener.dart

@@ -63,7 +63,11 @@ class IsolatePausedListener {
 
     // Resume the main isolate.
     if (_mainIsolate != null) {
-      await _service.resume(_mainIsolate!.id!);
+      try {
+        await _service.resume(_mainIsolate!.id!);
+      } on RPCError {
+        // The VM Service has already shut down, so there's nothing left to do.
+      }
     }
   }
 

pkgs/coverage/pubspec.yaml

@@ -1,5 +1,5 @@
 name: coverage
-version: 1.14.1-wip
+version: 1.14.1
 description: Coverage data manipulation and formatting
 repository: https://github.com/dart-lang/tools/tree/main/pkgs/coverage
 issue_tracker: https://github.com/dart-lang/tools/issues?q=is%3Aissue+is%3Aopen+label%3Apackage%3Acoverage

pkgs/coverage/test/isolate_paused_listener_test.dart

@@ -483,10 +483,12 @@ void main() {
     late MockVmService service;
     late StreamController<Event> allEvents;
     late Future<void> allIsolatesExited;
+    Object? lastError;
 
     late List<String> received;
     Future<void> Function(String)? delayTheOnPauseCallback;
     late bool stopped;
+    late Set<String> resumeFailures;
 
     void startEvent(String id, String groupId, [String? name]) =>
         allEvents.add(event(
@@ -516,34 +518,48 @@ void main() {
     }
 
     setUp(() {
-      (service, allEvents) = createServiceAndEventStreams();
-
-      // Backfill was tested above, so this test does everything using events,
-      // for simplicity. No need to report any isolates.
-      when(service.getVM()).thenAnswer((_) async => VM());
-
-      received = <String>[];
-      delayTheOnPauseCallback = null;
-      when(service.resume(any)).thenAnswer((invocation) async {
-        final id = invocation.positionalArguments[0];
-        received.add('Resume $id');
-        return Success();
-      });
-
-      stopped = false;
-      allIsolatesExited = IsolatePausedListener(
-        service,
-        (iso, isLastIsolateInGroup) async {
-          expect(stopped, isFalse);
-          received.add('Pause ${iso.id}. Collect group ${iso.isolateGroupId}? '
-              '${isLastIsolateInGroup ? 'Yes' : 'No'}');
-          if (delayTheOnPauseCallback != null) {
-            await delayTheOnPauseCallback!(iso.id!);
-            received.add('Pause done ${iso.id}');
+      Zone.current.fork(
+        specification: ZoneSpecification(
+          handleUncaughtError: (Zone self, ZoneDelegate parent, Zone zone,
+              Object error, StackTrace stackTrace) {
+            lastError = error;
+          },
+        ),
+      ).runGuarded(() {
+        (service, allEvents) = createServiceAndEventStreams();
+
+        // Backfill was tested above, so this test does everything using events,
+        // for simplicity. No need to report any isolates.
+        when(service.getVM()).thenAnswer((_) async => VM());
+
+        received = <String>[];
+        delayTheOnPauseCallback = null;
+        resumeFailures = <String>{};
+        when(service.resume(any)).thenAnswer((invocation) async {
+          final id = invocation.positionalArguments[0];
+          received.add('Resume $id');
+          if (resumeFailures.contains(id)) {
+            throw RPCError('resume', -32000, id);
           }
-        },
-        (message) => received.add(message),
-      ).waitUntilAllExited();
+          return Success();
+        });
+
+        stopped = false;
+        allIsolatesExited = IsolatePausedListener(
+          service,
+          (iso, isLastIsolateInGroup) async {
+            expect(stopped, isFalse);
+            received
+                .add('Pause ${iso.id}. Collect group ${iso.isolateGroupId}? '
+                    '${isLastIsolateInGroup ? 'Yes' : 'No'}');
+            if (delayTheOnPauseCallback != null) {
+              await delayTheOnPauseCallback!(iso.id!);
+              received.add('Pause done ${iso.id}');
+            }
+          },
+          (message) => received.add(message),
+        ).waitUntilAllExited();
+      });
     });
 
     test('ordinary flows', () async {
@@ -889,5 +905,40 @@ void main() {
         // Don't try to resume B, because the VM service is already shut down.
       ]);
     });
+
+    test('throw when resuming main isolate is ignored', () async {
+      resumeFailures = {'main'};
+
+      startEvent('main', '1');
+      startEvent('other', '2');
+      pauseEvent('other', '2');
+      exitEvent('other', '2');
+      pauseEvent('main', '1');
+      exitEvent('main', '1');
+
+      await endTest();
+      expect(lastError, isNull);
+
+      expect(received, [
+        'Pause other. Collect group 2? Yes',
+        'Resume other',
+        'Pause main. Collect group 1? Yes',
+        'Resume main',
+      ]);
+    });
+
+    test('throw when resuming other isolate is not ignored', () async {
+      resumeFailures = {'other'};
+
+      startEvent('main', '1');
+      startEvent('other', '2');
+      pauseEvent('other', '2');
+      exitEvent('other', '2');
+      pauseEvent('main', '1');
+      exitEvent('main', '1');
+
+      await endTest();
+      expect(lastError, isA<RPCError>());
+    });
   });
 }

pkgs/coverage/test/test_util.dart

@@ -11,7 +11,7 @@ import 'package:test_process/test_process.dart';
 
 final String testAppPath = p.join('test', 'test_files', 'test_app.dart');
 
-const Duration timeout = Duration(seconds: 30);
+const Duration timeout = Duration(seconds: 60);
 
 Future<TestProcess> runTestApp() => TestProcess.start(
       Platform.resolvedExecutable,