Await main in hot restart tests to fix some race conditions

- Recompile is needed for the frontend server even when no
edits are made
- Also use expectAll in hot_restart_breakpoints_test

Author: srujzs

dwds/test/common/hot_restart_common.dart

@@ -7,6 +7,8 @@
 @Timeout(Duration(minutes: 5))
 library;
 
+import 'dart:async';
+
 import 'package:dwds/dwds.dart';
 import 'package:dwds/expression_compiler.dart';
 import 'package:test/test.dart';
@@ -32,26 +34,54 @@ void runTests({
 
   tearDownAll(provider.dispose);
 
-  Future<void> makeEditAndRecompile() async {
-    context.makeEditToDartEntryFile(
-      toReplace: originalString,
-      replaceWith: newString,
-    );
+  Future<void> recompile({bool hasEdits = false}) async {
     if (compilationMode == CompilationMode.frontendServer) {
       await context.recompile(fullRestart: true);
     } else {
       assert(compilationMode == CompilationMode.buildDaemon);
-      await context.waitForSuccessfulBuild(propagateToBrowser: true);
+      if (hasEdits) {
+        // Only gets a new build if there were edits.
+        await context.waitForSuccessfulBuild(propagateToBrowser: true);
+      }
     }
   }
 
+  Future<void> makeEditAndRecompile() async {
+    context.makeEditToDartEntryFile(
+      toReplace: originalString,
+      replaceWith: newString,
+    );
+    await recompile(hasEdits: true);
+  }
+
   void undoEdit() {
     context.makeEditToDartEntryFile(
       toReplace: newString,
       replaceWith: originalString,
     );
   }
 
+  /// Wait for main to finish executing before checking expectations by checking
+  /// for a log output.
+  ///
+  /// If [debuggingEnabled] is false, we can't check for Chrome logs and instead
+  /// wait 1 second.
+  // TODO(srujzs): We should do something less prone to race conditions when
+  // debugging is disabled.
+  Future<void> waitForMainToExecute({bool debuggingEnabled = true}) async {
+    if (!debuggingEnabled) return Future.delayed(const Duration(seconds: 1));
+    final completer = Completer<void>();
+    final expectedString = 'main executed';
+    final subscription = context.webkitDebugger.onConsoleAPICalled.listen((e) {
+      if (e.args.first.value == expectedString) {
+        completer.complete();
+      }
+    });
+    await completer.future.then((_) {
+      subscription.cancel();
+    });
+  }
+
   group(
     'Injected client with live reload',
     () {
@@ -74,7 +104,9 @@ void runTests({
         });
 
         test('can live reload changes ', () async {
+          final mainDone = waitForMainToExecute();
           await makeEditAndRecompile();
+          await mainDone;
           final source = await context.webDriver.pageSource;
 
           // A full reload should clear the state.
@@ -105,8 +137,9 @@ void runTests({
         });
 
         test('can live reload changes ', () async {
+          final mainDone = waitForMainToExecute(debuggingEnabled: false);
           await makeEditAndRecompile();
-
+          await mainDone;
           final source = await context.webDriver.pageSource;
 
           // A full reload should clear the state.
@@ -138,8 +171,9 @@ void runTests({
         });
 
         test('can live reload changes ', () async {
+          final mainDone = waitForMainToExecute(debuggingEnabled: false);
           await makeEditAndRecompile();
-
+          await mainDone;
           final source = await context.webDriver.pageSource;
 
           // A full reload should clear the state.
@@ -281,13 +315,15 @@ void runTests({
           ]),
         ),
       );
+      final mainDone = waitForMainToExecute();
       final hotRestart = context.getRegisteredServiceExtension('hotRestart');
       expect(
         await fakeClient.callServiceExtension(hotRestart!),
         const TypeMatcher<Success>(),
       );
 
       await eventsDone;
+      await mainDone;
 
       final source = await context.webDriver.pageSource;
       // Main is re-invoked which shouldn't clear the state.
@@ -324,6 +360,8 @@ void runTests({
         "registerExtension('ext.foo', $callback)",
       );
 
+      await recompile();
+      final mainDone = waitForMainToExecute();
       final hotRestart = context.getRegisteredServiceExtension('hotRestart');
       expect(
         await fakeClient.callServiceExtension(hotRestart!),
@@ -342,6 +380,7 @@ void runTests({
       );
 
       await eventsDone;
+      await mainDone;
 
       final source = await context.webDriver.pageSource;
       // Main is re-invoked which shouldn't clear the state.
@@ -363,14 +402,15 @@ void runTests({
           ]),
         ),
       );
-
+      final mainDone = waitForMainToExecute();
       final fullReload = context.getRegisteredServiceExtension('fullReload');
       expect(
         await fakeClient.callServiceExtension(fullReload!),
         isA<Success>(),
       );
 
       await eventsDone;
+      await mainDone;
 
       final source = await context.webDriver.pageSource;
       // Should see only the new text
@@ -399,8 +439,12 @@ void runTests({
       );
 
       await makeEditAndRecompile();
+      final mainDone = waitForMainToExecute();
       final hotRestart = context.getRegisteredServiceExtension('hotRestart');
       await fakeClient.callServiceExtension(hotRestart!);
+
+      await mainDone;
+
       final source = await context.webDriver.pageSource;
 
       // Main is re-invoked which shouldn't clear the state.
@@ -418,6 +462,7 @@ void runTests({
     test('can evaluate expressions after hot restart', () async {
       final client = context.debugConnection.vmService;
 
+      await recompile();
       final hotRestart = context.getRegisteredServiceExtension('hotRestart');
       await fakeClient.callServiceExtension(hotRestart!);
 
@@ -461,8 +506,9 @@ void runTests({
         });
 
         test('can hot restart changes ', () async {
+          final mainDone = waitForMainToExecute();
           await makeEditAndRecompile();
-
+          await mainDone;
           final source = await context.webDriver.pageSource;
 
           // Main is re-invoked which shouldn't clear the state.
@@ -521,8 +567,9 @@ void runTests({
         });
 
         test('can hot restart changes ', () async {
+          final mainDone = waitForMainToExecute(debuggingEnabled: false);
           await makeEditAndRecompile();
-
+          await mainDone;
           final source = await context.webDriver.pageSource;
 
           // Main is re-invoked which shouldn't clear the state.
@@ -582,6 +629,7 @@ void runTests({
           ),
         );
 
+        final mainDone = waitForMainToExecute();
         final hotRestart = context.getRegisteredServiceExtension('hotRestart');
         expect(
           await fakeClient.callServiceExtension(hotRestart!),
@@ -597,6 +645,8 @@ void runTests({
         final isolateId = vm.isolates!.first.id!;
         await client.resume(isolateId);
 
+        await mainDone;
+
         final sourceAfterResume = await context.webDriver.pageSource;
         expect(sourceAfterResume.contains(newString), isTrue);
       },
@@ -605,8 +655,8 @@ void runTests({
     test(
       'after page refresh, does not run app until there is a resume event',
       () async {
+        final mainDone = waitForMainToExecute();
         await makeEditAndRecompile();
-
         await context.webDriver.driver.refresh();
 
         final eventsDone = expectLater(
@@ -629,6 +679,8 @@ void runTests({
         final isolateId = vm.isolates!.first.id!;
         await client.resume(isolateId);
 
+        await mainDone;
+
         final sourceAfterResume = await context.webDriver.pageSource;
         expect(sourceAfterResume.contains(newString), isTrue);
       },

dwds/test/hot_restart_breakpoints_test.dart

@@ -181,8 +181,9 @@ void main() {
         ),
       );
 
-      final waitForPausePost = stream.firstWhere(
-        (event) => event.kind == EventKind.kPausePostRequest,
+      final waitForPausePost = expectLater(
+        stream,
+        emitsThrough(_hasKind(EventKind.kPausePostRequest)),
       );
 
       final hotRestart = context.getRegisteredServiceExtension('hotRestart');

fixtures/_testSound/example/append_body/main.dart

@@ -7,6 +7,10 @@ import 'dart:developer';
 // TODO: https://github.com/dart-lang/webdev/issues/2508
 // ignore: deprecated_member_use
 import 'dart:html';
+import 'dart:js_interop';
+
+@JS('console.log')
+external void log(String _);
 
 void main() {
   var count = 0;
@@ -23,4 +27,7 @@ void main() {
     document.body?.appendText('end disassemble ');
     return ServiceExtensionResponse.result('{}');
   });
+
+  // Wait for this print statement so that we know main is done executing.
+  log('main executed');
 }