[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'));
+}