[dartdevc] Refactoring expression eval tests.

* Splitting expression eval tests into separate tests per module system to avoid timeouts
* Adding some extra timeouts to various Chrome operations in the e2e suite
* Running in non-headless Chrome on Windows to avoid strange flakiness problems
* Minor test suite reformatting

For timeouts, see: https://dart-review.googlesource.com/c/sdk/+/208347/21..22

Fixes #45713

Change-Id: Id51d412da1ffc66b443c73cdb50daa777bdce929
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/207562
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Anna Gringauze <annagrin@google.com>
Commit-Queue: Mark Zhou <markzipan@google.com>
diff --git a/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_sound_test.dart b/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_amd_sound_test.dart
similarity index 76%
copy from pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_sound_test.dart
copy to pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_amd_sound_test.dart
index 9db3279..9b56991 100644
--- a/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_sound_test.dart
+++ b/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_amd_sound_test.dart
@@ -15,8 +15,8 @@
   var driver = await TestDriver.init();
 
   group('(Sound null safety)', () {
-    tearDownAll(() {
-      driver.finish();
+    tearDownAll(() async {
+      await driver.finish();
     });
 
     group('(AMD module system)', () {
@@ -24,11 +24,5 @@
           soundNullSafety: true, moduleFormat: ModuleFormat.amd);
       runSharedTests(setup, driver);
     });
-
-    group('(DDC module system)', () {
-      var setup = SetupCompilerOptions(
-          soundNullSafety: true, moduleFormat: ModuleFormat.ddc);
-      runSharedTests(setup, driver);
-    });
   });
 }
diff --git a/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_unsound_test.dart b/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_amd_unsound_test.dart
similarity index 76%
copy from pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_unsound_test.dart
copy to pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_amd_unsound_test.dart
index 72df6f5..92a7c96 100644
--- a/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_unsound_test.dart
+++ b/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_amd_unsound_test.dart
@@ -15,8 +15,8 @@
   var driver = await TestDriver.init();
 
   group('(Unsound null safety)', () {
-    tearDownAll(() {
-      driver.finish();
+    tearDownAll(() async {
+      await driver.finish();
     });
 
     group('(AMD module system)', () {
@@ -24,11 +24,5 @@
           soundNullSafety: false, moduleFormat: ModuleFormat.amd);
       runSharedTests(setup, driver);
     });
-
-    group('(DDC module system)', () {
-      var setup = SetupCompilerOptions(
-          soundNullSafety: false, moduleFormat: ModuleFormat.ddc);
-      runSharedTests(setup, driver);
-    });
   });
 }
diff --git a/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_sound_test.dart b/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_ddc_sound_test.dart
similarity index 76%
rename from pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_sound_test.dart
rename to pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_ddc_sound_test.dart
index 9db3279..682327e 100644
--- a/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_sound_test.dart
+++ b/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_ddc_sound_test.dart
@@ -15,14 +15,8 @@
   var driver = await TestDriver.init();
 
   group('(Sound null safety)', () {
-    tearDownAll(() {
-      driver.finish();
-    });
-
-    group('(AMD module system)', () {
-      var setup = SetupCompilerOptions(
-          soundNullSafety: true, moduleFormat: ModuleFormat.amd);
-      runSharedTests(setup, driver);
+    tearDownAll(() async {
+      await driver.finish();
     });
 
     group('(DDC module system)', () {
diff --git a/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_unsound_test.dart b/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_ddc_unsound_test.dart
similarity index 76%
rename from pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_unsound_test.dart
rename to pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_ddc_unsound_test.dart
index 72df6f5..21bc460 100644
--- a/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_unsound_test.dart
+++ b/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_ddc_unsound_test.dart
@@ -15,14 +15,8 @@
   var driver = await TestDriver.init();
 
   group('(Unsound null safety)', () {
-    tearDownAll(() {
-      driver.finish();
-    });
-
-    group('(AMD module system)', () {
-      var setup = SetupCompilerOptions(
-          soundNullSafety: false, moduleFormat: ModuleFormat.amd);
-      runSharedTests(setup, driver);
+    tearDownAll(() async {
+      await driver.finish();
     });
 
     group('(DDC module system)', () {
diff --git a/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_shared.dart b/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_shared.dart
index 40f3f36..18526d1 100644
--- a/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_shared.dart
+++ b/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_shared.dart
@@ -173,8 +173,8 @@
       await driver.initSource(setup, source);
     });
 
-    tearDownAll(() {
-      driver.cleanupTest();
+    tearDownAll(() async {
+      await driver.cleanupTest();
     });
 
     test('compilation error', () async {
@@ -215,8 +215,8 @@
       await driver.initSource(setup, source);
     });
 
-    tearDownAll(() {
-      driver.cleanupTest();
+    tearDownAll(() async {
+      await driver.cleanupTest();
     });
 
     test('compilation error', () async {
@@ -257,8 +257,8 @@
       await driver.initSource(setup, source);
     });
 
-    tearDownAll(() {
-      driver.cleanupTest();
+    tearDownAll(() async {
+      await driver.cleanupTest();
     });
 
     test('compilation error', () async {
@@ -366,8 +366,8 @@
       await driver.initSource(setup, source);
     });
 
-    tearDownAll(() {
-      driver.cleanupTest();
+    tearDownAll(() async {
+      await driver.cleanupTest();
     });
 
     test('compilation error', () async {
@@ -501,8 +501,8 @@
       await driver.initSource(setup, source);
     });
 
-    tearDownAll(() {
-      driver.cleanupTest();
+    tearDownAll(() async {
+      await driver.cleanupTest();
     });
 
     test('compilation error', () async {
@@ -649,8 +649,8 @@
       await driver.initSource(setup, source);
     });
 
-    tearDownAll(() {
-      driver.cleanupTest();
+    tearDownAll(() async {
+      await driver.cleanupTest();
     });
 
     test('compilation error', () async {
@@ -712,8 +712,8 @@
       await driver.initSource(setup, source);
     });
 
-    tearDownAll(() {
-      driver.cleanupTest();
+    tearDownAll(() async {
+      await driver.cleanupTest();
     });
 
     test('compilation error', () async {
@@ -780,8 +780,8 @@
       await driver.initSource(setup, source);
     });
 
-    tearDownAll(() {
-      driver.cleanupTest();
+    tearDownAll(() async {
+      await driver.cleanupTest();
     });
 
     test('call function not using type', () async {
@@ -840,8 +840,8 @@
       await driver.initSource(setup, source);
     });
 
-    tearDownAll(() {
-      driver.cleanupTest();
+    tearDownAll(() async {
+      await driver.cleanupTest();
     });
 
     test('expression using local', () async {
@@ -881,8 +881,8 @@
       await driver.initSource(setup, source);
     });
 
-    tearDownAll(() {
-      driver.cleanupTest();
+    tearDownAll(() async {
+      await driver.cleanupTest();
     });
 
     test('(then) expression using local', () async {
@@ -948,8 +948,8 @@
       await driver.initSource(setup, source);
     });
 
-    tearDownAll(() {
-      driver.cleanupTest();
+    tearDownAll(() async {
+      await driver.cleanupTest();
     });
 
     test('expression loop variable', () async {
@@ -975,8 +975,8 @@
       await driver.initSource(setup, source);
     });
 
-    tearDownAll(() {
-      driver.cleanupTest();
+    tearDownAll(() async {
+      await driver.cleanupTest();
     });
 
     test('evaluate formals', () async {
@@ -1020,8 +1020,8 @@
       await driver.initSource(setup, source);
     });
 
-    tearDownAll(() {
-      driver.cleanupTest();
+    tearDownAll(() async {
+      await driver.cleanupTest();
     });
 
     test('evaluation that non-destructively appends to the type container',
diff --git a/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_suite.dart b/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_suite.dart
index 7300029..a20eb79 100644
--- a/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_suite.dart
+++ b/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_suite.dart
@@ -107,22 +107,23 @@
 
 class TestCompiler {
   final SetupCompilerOptions setup;
-  Component component;
-  ExpressionCompiler evaluator;
-  ModuleMetadata metadata;
-  source_maps.SingleMapping sourceMap;
+  final Component component;
+  final ExpressionCompiler evaluator;
+  final ModuleMetadata metadata;
+  final source_maps.SingleMapping sourceMap;
 
-  TestCompiler(this.setup);
+  TestCompiler._(this.setup, this.component, this.evaluator, this.metadata,
+      this.sourceMap);
 
-  Future<TestCompiler> init({Uri input, Uri output, Uri packages}) async {
+  static Future<TestCompiler> init(SetupCompilerOptions setup,
+      {Uri input, Uri output, Uri packages}) async {
     // Initialize the incremental compiler and module component.
     // TODO: extend this for multi-module compilations by storing separate
     // compilers/components/names per module.
     setup.options.packagesFileUri = packages;
     var compiler = DevelopmentIncrementalCompiler(setup.options, input);
-    component = await compiler.computeDelta();
+    var component = await compiler.computeDelta();
     component.computeCanonicalNames();
-
     // Initialize DDC.
     var moduleName = '${p.basenameWithoutExtension(output.toFilePath())}';
 
@@ -158,8 +159,6 @@
       compiler: kernel2jsCompiler,
       component: component,
     );
-    metadata = code.metadata;
-    sourceMap = source_maps.SingleMapping.fromJson(code.sourceMap);
     var codeBytes = utf8.encode(code.code);
     var sourceMapBytes = utf8.encode(json.encode(code.sourceMap));
 
@@ -167,7 +166,7 @@
     File('${output.toFilePath()}.map').writeAsBytesSync(sourceMapBytes);
 
     // Save the expression evaluator for future evaluations.
-    evaluator = ExpressionCompiler(
+    var evaluator = ExpressionCompiler(
       setup.options,
       setup.moduleFormat,
       setup.errors,
@@ -181,7 +180,9 @@
     }
     setup.diagnosticMessages.clear();
 
-    return this;
+    var sourceMap = source_maps.SingleMapping.fromJson(code.sourceMap);
+    return TestCompiler._(
+        setup, component, evaluator, code.metadata, sourceMap);
   }
 
   Future<TestCompilationResult> compileExpression(
@@ -212,45 +213,61 @@
 }
 
 class TestDriver {
-  SetupCompilerOptions setup;
-  String source;
-  Directory chromeDir;
-  Directory testDir;
-  String moduleFormatString;
+  final browser.Chrome chrome;
+  final Directory chromeDir;
+  final wip.WipConnection connection;
+  final wip.WipDebugger debugger;
+  TestCompiler compiler;
   Uri htmlBootstrapper;
   Uri input;
+  String moduleFormatString;
   Uri output;
   Uri packagesFile;
-  browser.Chrome chrome;
-  wip.WipDebugger debugger;
-  wip.WipConnection connection;
-  TestCompiler compiler;
+  String preemptiveBp;
+  SetupCompilerOptions setup;
+  String source;
+  Directory testDir;
 
-  TestDriver._();
-
-  static Future<TestDriver> init() async {
-    var driver = TestDriver._();
-    await driver.initChrome();
-    return driver;
-  }
+  TestDriver._(this.chrome, this.chromeDir, this.connection, this.debugger);
 
   /// Initializes a Chrome browser instance, tab connection, and debugger.
-  ///
-  /// Should be called once after creating TestDriver.
-  Future<void> initChrome() async {
+  static Future<TestDriver> init() async {
     // Create a temporary directory for holding Chrome tests.
-    var systemTempDir = Directory.systemTemp;
-    chromeDir = await systemTempDir.createTemp('ddc_eval_test_anchor');
+    var chromeDir = Directory.systemTemp.createTempSync('ddc_eval_test_anchor');
 
-    // Start Chrome on an empty page with a single empty tab.
-    chrome = await browser.Chrome.startWithDebugPort(['about:blank'],
-        userDataDir: chromeDir.uri.toFilePath(), headless: true);
+    // Try to start Chrome on an empty page with a single empty tab.
+    // TODO(#45713): Headless Chrome crashes the Windows bots, so run in
+    // standard mode until it's fixed.
+    browser.Chrome chrome;
+    var retries = 3;
+    while (chrome == null && retries-- > 0) {
+      try {
+        chrome = await browser.Chrome.startWithDebugPort(['about:blank'],
+            userDataDir: chromeDir.uri.toFilePath(),
+            headless: !Platform.isWindows);
+      } catch (e) {
+        if (retries == 0) rethrow;
+        await Future.delayed(Duration(seconds: 5));
+      }
+    }
 
     // Connect to the first 'normal' tab.
     var tab = await chrome.chromeConnection
         .getTab((tab) => !tab.isBackgroundPage && !tab.isChromeExtension);
-    connection = await tab.connect();
-    debugger = connection.debugger;
+    if (tab == null) {
+      throw Exception('Unable to connect to Chrome tab');
+    }
+
+    var connection = await tab.connect().timeout(Duration(seconds: 5),
+        onTimeout: () => throw Exception('Unable to connect to WIP tab'));
+
+    await connection.page.enable().timeout(Duration(seconds: 5),
+        onTimeout: () => throw Exception('Unable to enable WIP tab page'));
+
+    var debugger = connection.debugger;
+    await debugger.enable().timeout(Duration(seconds: 5),
+        onTimeout: () => throw Exception('Unable to enable WIP debugger'));
+    return TestDriver._(chrome, chromeDir, connection, debugger);
   }
 
   /// Must be called when testing a new Dart program.
@@ -268,7 +285,7 @@
     source = '${setup.dartLangComment}\n\n$source';
     this.setup = setup;
     this.source = source;
-    testDir = await chromeDir.createTemp('ddc_eval_test');
+    testDir = chromeDir.createTempSync('ddc_eval_test');
     var buildDir = p.dirname(p.dirname(p.dirname(Platform.resolvedExecutable)));
     var scriptPath = Platform.script.normalizePath().toFilePath();
     var ddcPath = p.dirname(p.dirname(p.dirname(scriptPath)));
@@ -295,8 +312,8 @@
       ''');
 
     // Initialize DDC and the incremental compiler, then perform a full compile.
-    compiler = await TestCompiler(setup)
-        .init(input: input, output: output, packages: packagesFile);
+    compiler = await TestCompiler.init(setup,
+        input: input, output: output, packages: packagesFile);
 
     htmlBootstrapper = testDir.uri.resolve('bootstrapper.html');
     var bootstrapFile = File(htmlBootstrapper.toFilePath())..createSync();
@@ -314,6 +331,9 @@
             setup.soundNullSafety ? 'sound' : 'kernel',
             'legacy',
             'dart_sdk.js'));
+        if (!File(dartSdkPath).existsSync()) {
+          throw Exception('Unable to find Dart SDK at $dartSdkPath');
+        }
         var dartLibraryPath =
             escaped(p.join(ddcPath, 'lib', 'js', 'legacy', 'dart_library.js'));
         var outputPath = output.toFilePath();
@@ -341,6 +361,9 @@
         moduleFormatString = 'amd';
         var dartSdkPath = escaped(p.join(buildDir, 'gen', 'utils', 'dartdevc',
             setup.soundNullSafety ? 'sound' : 'kernel', 'amd', 'dart_sdk'));
+        if (!File('$dartSdkPath.js').existsSync()) {
+          throw Exception('Unable to find Dart SDK at $dartSdkPath.js');
+        }
         var requirePath = escaped(p.join(buildDir, 'dart-sdk', 'lib',
             'dev_compiler', 'kernel', 'amd', 'require.js'));
         var outputPath = escaped(p.withoutExtension(output.toFilePath()));
@@ -378,27 +401,30 @@
             'Unsupported module format for SDK evaluation tests: ${setup.moduleFormat}');
     }
 
-    await debugger.enable();
+    await setBreakpointsActive(debugger, true);
 
     // Pause as soon as the test file loads but before it executes.
     var urlRegex = '.*${libraryUriToJsIdentifier(output)}.*';
-    await debugger.sendCommand('Debugger.setBreakpointByUrl', params: {
+    var bpResponse =
+        await debugger.sendCommand('Debugger.setBreakpointByUrl', params: {
       'urlRegex': urlRegex,
       'lineNumber': 0,
     });
+    preemptiveBp = wip.SetBreakpointResponse(bpResponse.json).breakpointId;
   }
 
-  void finish() async {
+  Future<void> finish() async {
     await chrome?.close();
     // Chrome takes a while to free its claim on chromeDir, so wait a bit.
-    await Future.delayed(const Duration(milliseconds: 500));
+    await Future.delayed(Duration(milliseconds: 500));
     chromeDir?.deleteSync(recursive: true);
   }
 
   Future<void> cleanupTest() async {
+    await setBreakpointsActive(debugger, false);
+    await debugger.removeBreakpoint(preemptiveBp);
     setup.diagnosticMessages.clear();
     setup.errors.clear();
-    await debugger.disable();
   }
 
   Future<void> check(
@@ -424,11 +450,14 @@
 
     // Navigate from the empty page and immediately pause on the preemptive
     // breakpoint.
-    await connection.page.navigate('$htmlBootstrapper');
+    await connection.page.navigate('$htmlBootstrapper').timeout(
+        Duration(seconds: 5),
+        onTimeout: () => throw Exception(
+            'Unable to navigate to page bootstrap script: $htmlBootstrapper'));
 
     // Poll until the script is found, or timeout after a few seconds.
     var script = (await scriptController.stream.first.timeout(
-            const Duration(seconds: 5),
+            Duration(seconds: 5),
             onTimeout: () => throw Exception(
                 'Unable to find JS script corresponding to test file $output in ${debugger.scripts}.')))
         .script;
@@ -446,11 +475,11 @@
     await debugger.resume();
     final event = await pauseController.stream
         .skip(1)
-        .timeout(const Duration(seconds: 5),
+        .timeout(Duration(seconds: 5),
             onTimeout: (event) => throw Exception(
                 'Unable to find JS preemptive pause event in $output.'))
         .first
-        .timeout(const Duration(seconds: 5),
+        .timeout(Duration(seconds: 5),
             onTimeout: () => throw Exception(
                 'Unable to find JS pause event corresponding to line ($dartLine -> $location) in $output.'));
     await pauseSub.cancel();
@@ -490,6 +519,9 @@
     await debugger.removeBreakpoint(bp.breakpointId);
     var value = await stringifyRemoteObject(evalResult);
 
+    // Resume execution to the end of the current script
+    await debugger.resume();
+
     expect(
         result,
         const TypeMatcher<TestCompilationResult>()
@@ -614,3 +646,10 @@
 }
 
 String escaped(String path) => path.replaceAll('\\', '\\\\');
+
+Future setBreakpointsActive(wip.WipDebugger debugger, bool active) async {
+  await debugger.sendCommand('Debugger.setBreakpointsActive', params: {
+    'active': active
+  }).timeout(Duration(seconds: 5),
+      onTimeout: () => throw Exception('Unable to set breakpoint activity'));
+}