diff --git a/pkg/front_end/test/testing_utils.dart b/pkg/front_end/test/testing_utils.dart index ba228a052db..6781673c2f4 100644 --- a/pkg/front_end/test/testing_utils.dart +++ b/pkg/front_end/test/testing_utils.dart @@ -26,6 +26,11 @@ Future> getGitFiles(Uri uri) async { ProcessResult result = await Process.run("git", ["ls-files", "."], workingDirectory: new Directory.fromUri(uri).absolute.path, runInShell: true); + if (result.exitCode != 0) { + throw "Git returned non-zero error code (${result.exitCode}):\n\n" + "stdout: ${result.stdout}\n\n" + "stderr: ${result.stderr}"; + } String stdout = result.stdout; return stdout .split(new RegExp('^', multiLine: true)) diff --git a/pkg/front_end/test/unit_test_suites_impl.dart b/pkg/front_end/test/unit_test_suites_impl.dart index fde99ac07c2..54af94b9c06 100644 --- a/pkg/front_end/test/unit_test_suites_impl.dart +++ b/pkg/front_end/test/unit_test_suites_impl.dart @@ -2,7 +2,7 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE.md file. -// @dart = 2.9 +// ignore_for_file: import_of_legacy_library_into_null_safe import 'dart:async' show Timer; import 'dart:convert' show jsonEncode; @@ -40,16 +40,40 @@ import 'spelling_test_src_suite.dart' as spelling_src show createContext; const suiteNamePrefix = "pkg/front_end/test"; +int getDefaultThreads() { + int numberOfWorkers = 1; + if (Platform.numberOfProcessors > 2) { + numberOfWorkers = Platform.numberOfProcessors - 1; + } + return numberOfWorkers; +} + class Options { final String configurationName; final bool verbose; final bool printFailureLog; final Uri outputDirectory; - final String testFilter; + final String? testFilter; final List environmentOptions; + final int shardCount; + final int shard; + final bool skipTestsThatRequireGit; + final bool onlyTestsThatRequireGit; + final int numberOfWorkers; - Options(this.configurationName, this.verbose, this.printFailureLog, - this.outputDirectory, this.testFilter, this.environmentOptions); + Options( + this.configurationName, + this.verbose, + this.printFailureLog, + this.outputDirectory, + this.testFilter, + this.environmentOptions, { + required this.shardCount, + required this.shard, + required this.skipTestsThatRequireGit, + required this.onlyTestsThatRequireGit, + required this.numberOfWorkers, + }); static Options parse(List args) { var parser = new ArgParser() @@ -63,24 +87,83 @@ class Options { ..addFlag("print", abbr: "p", help: "print failure logs", defaultsTo: false) ..addMultiOption('environment', - abbr: 'D', help: "environment options for the test suite"); + abbr: 'D', help: "environment options for the test suite") + ..addOption("tasks", + abbr: "j", + help: "The number of parallel tasks to run.", + defaultsTo: "${getDefaultThreads()}") + ..addOption("shards", help: "Number of shards", defaultsTo: "1") + ..addOption("shard", help: "Which shard to run", defaultsTo: "1") + ..addFlag("skipTestsThatRequireGit", + help: "Whether to skip tests that require git to run", + defaultsTo: false) + ..addFlag("onlyTestsThatRequireGit", + help: "Whether to only run tests that require git", + defaultsTo: false); var parsedArguments = parser.parse(args); String outputPath = parsedArguments["output-directory"] ?? "."; Uri outputDirectory = Uri.base.resolveUri(Uri.directory(outputPath)); - String filter; + + bool verbose = parsedArguments["verbose"]; + + String? filter; if (parsedArguments.rest.length == 1) { filter = parsedArguments.rest.single; if (filter.startsWith("$suiteNamePrefix/")) { filter = filter.substring(suiteNamePrefix.length + 1); } } + String tasksString = parsedArguments["tasks"]; + int? tasks = int.tryParse(tasksString); + if (tasks == null || tasks < 1) { + throw "--tasks (-j) has to be an integer >= 1"; + } + + String shardsString = parsedArguments["shards"]; + int? shardCount = int.tryParse(shardsString); + if (shardCount == null || shardCount < 1) { + throw "--shards has to be an integer >= 1"; + } + String shardString = parsedArguments["shard"]; + int? shard = int.tryParse(shardString); + if (shard == null || shard < 1 || shard > shardCount) { + throw "--shard has to be an integer >= 1 (and <= --shards)"; + } + bool skipTestsThatRequireGit = parsedArguments["skipTestsThatRequireGit"]; + bool onlyTestsThatRequireGit = parsedArguments["onlyTestsThatRequireGit"]; + if (skipTestsThatRequireGit && onlyTestsThatRequireGit) { + throw "Only one of --skipTestsThatRequireGit and " + "--onlyTestsThatRequireGit can be provided."; + } + + if (verbose) { + print("NOTE: Created with options\n " + "${parsedArguments["named-configuration"]},\n " + "${verbose},\n " + "${parsedArguments["print"]},\n " + "${outputDirectory},\n " + "${filter},\n " + "${parsedArguments['environment']},\n " + "shardCount: ${shardCount},\n " + "shard: ${shard - 1 /* make it 0-indexed */},\n " + "onlyTestsThatRequireGit: ${onlyTestsThatRequireGit},\n " + "skipTestsThatRequireGit: ${skipTestsThatRequireGit},\n " + "numberOfWorkers: ${tasks}"); + } + return Options( - parsedArguments["named-configuration"], - parsedArguments["verbose"], - parsedArguments["print"], - outputDirectory, - filter, - parsedArguments['environment']); + parsedArguments["named-configuration"], + verbose, + parsedArguments["print"], + outputDirectory, + filter, + parsedArguments['environment'], + shardCount: shardCount, + shard: shard - 1 /* make it 0-indexed */, + onlyTestsThatRequireGit: onlyTestsThatRequireGit, + skipTestsThatRequireGit: skipTestsThatRequireGit, + numberOfWorkers: tasks, + ); } } @@ -93,6 +176,7 @@ class ResultLogger implements Logger { final Map stopwatches = {}; final String configurationName; final Set seenTests = {}; + bool gotFrameworkError = false; ResultLogger(this.prefix, this.resultsPort, this.logsPort, this.verbose, this.printFailureLog, this.configurationName); @@ -134,7 +218,7 @@ class ResultLogger implements Logger { "configuration": configurationName, "suite": suite, "test_name": shortTestName, - "time_ms": stopwatches[testName].elapsedMilliseconds, + "time_ms": stopwatches[testName]!.elapsedMilliseconds, "expected": "Pass", "result": matchedExpectations ? "Pass" : "Fail", "matches": matchedExpectations, @@ -211,116 +295,169 @@ class ResultLogger implements Logger { seenTests.add(testName); handleTestResult(description, result, prefix, false); } + + @override + void noticeFrameworkCatchError(error, StackTrace stackTrace) { + gotFrameworkError = true; + } } class Suite { - final String name; final CreateContext createContext; final String testingRootPath; - final String path; + final String? path; final int shardCount; - final int shard; final String prefix; + final bool requiresGit; - const Suite(this.name, this.createContext, this.testingRootPath, - {this.path, this.shardCount: 1, this.shard: 0, String prefix}) - : prefix = prefix ?? name; + const Suite( + this.prefix, + this.createContext, + this.testingRootPath, { + required this.shardCount, + this.path, + this.requiresGit: false, + }); } const List suites = [ const Suite( - "fasta/expression", expression.createContext, "../../testing.json"), - const Suite("fasta/outline", outline.createContext, "../../testing.json"), + "fasta/expression", + expression.createContext, + "../../testing.json", + shardCount: 1, + ), const Suite( - "fasta/fast_strong", fast_strong.createContext, "../../testing.json"), + "fasta/outline", + outline.createContext, + "../../testing.json", + shardCount: 2, + ), + // TODO(CFE TEAM): Should the 'fast strong' be run at all? const Suite( - "fasta/incremental", incremental.createContext, "../../testing.json"), - const Suite("fasta/messages", messages.createContext, "../../testing.json"), - const Suite("fasta/text_serialization1", text_serialization.createContext, - "../../testing.json", - path: "fasta/text_serialization_tester.dart", - shardCount: 4, - shard: 0, - prefix: "fasta/text_serialization"), - const Suite("fasta/text_serialization2", text_serialization.createContext, - "../../testing.json", - path: "fasta/text_serialization_tester.dart", - shardCount: 4, - shard: 1, - prefix: "fasta/text_serialization"), - const Suite("fasta/text_serialization3", text_serialization.createContext, - "../../testing.json", - path: "fasta/text_serialization_tester.dart", - shardCount: 4, - shard: 2, - prefix: "fasta/text_serialization"), - const Suite("fasta/text_serialization4", text_serialization.createContext, - "../../testing.json", - path: "fasta/text_serialization_tester.dart", - shardCount: 4, - shard: 3, - prefix: "fasta/text_serialization"), - const Suite("fasta/strong1", strong.createContext, "../../testing.json", - path: "fasta/strong_tester.dart", - shardCount: 4, - shard: 0, - prefix: "fasta/strong"), - const Suite("fasta/strong2", strong.createContext, "../../testing.json", - path: "fasta/strong_tester.dart", - shardCount: 4, - shard: 1, - prefix: "fasta/strong"), - const Suite("fasta/strong3", strong.createContext, "../../testing.json", - path: "fasta/strong_tester.dart", - shardCount: 4, - shard: 2, - prefix: "fasta/strong"), - const Suite("fasta/strong4", strong.createContext, "../../testing.json", - path: "fasta/strong_tester.dart", - shardCount: 4, - shard: 3, - prefix: "fasta/strong"), - const Suite("incremental_bulk_compiler_smoke", - incremental_bulk_compiler.createContext, "../testing.json"), - const Suite("incremental_load_from_dill", incremental_load.createContext, - "../testing.json"), - const Suite("lint", lint.createContext, "../testing.json"), - const Suite("parser", parser.createContext, "../testing.json"), - const Suite("parser_all", parserAll.createContext, "../testing.json"), - const Suite("spelling_test_not_src", spelling_not_src.createContext, - "../testing.json"), + "fasta/fast_strong", + fast_strong.createContext, + "../../testing.json", + shardCount: 4, + ), const Suite( - "spelling_test_src", spelling_src.createContext, "../testing.json"), - const Suite("fasta/weak", weak.createContext, "../../testing.json"), - const Suite("fasta/textual_outline", textual_outline.createContext, - "../../testing.json"), + "fasta/incremental", + incremental.createContext, + "../../testing.json", + shardCount: 1, + ), + const Suite( + "fasta/messages", + messages.createContext, + "../../testing.json", + shardCount: 1, + requiresGit: true, + ), + const Suite( + "fasta/text_serialization", + text_serialization.createContext, + "../../testing.json", + path: "fasta/text_serialization_tester.dart", + shardCount: 10, + ), + const Suite( + "fasta/strong", + strong.createContext, + "../../testing.json", + path: "fasta/strong_tester.dart", + shardCount: 10, + ), + const Suite( + "incremental_bulk_compiler_smoke", + incremental_bulk_compiler.createContext, + "../testing.json", + shardCount: 1, + ), + const Suite( + "incremental_load_from_dill", + incremental_load.createContext, + "../testing.json", + shardCount: 2, + ), + const Suite( + "lint", + lint.createContext, + "../testing.json", + shardCount: 1, + requiresGit: true, + ), + const Suite( + "parser", + parser.createContext, + "../testing.json", + shardCount: 1, + ), + const Suite( + "parser_all", + parserAll.createContext, + "../testing.json", + shardCount: 4, + requiresGit: + true /* technically not true, but tests *many* more files + than in test_matrix.json file set */ + , + ), + const Suite( + "spelling_test_not_src", + spelling_not_src.createContext, + "../testing.json", + shardCount: 1, + requiresGit: true, + ), + const Suite( + "spelling_test_src", + spelling_src.createContext, + "../testing.json", + shardCount: 1, + requiresGit: true, + ), + const Suite( + "fasta/weak", + weak.createContext, + "../../testing.json", + shardCount: 2, + ), + const Suite( + "fasta/textual_outline", + textual_outline.createContext, + "../../testing.json", + shardCount: 1, + ), ]; const Duration timeoutDuration = Duration(minutes: 30); class SuiteConfiguration { - final String name; + final Suite suite; final SendPort resultsPort; final SendPort logsPort; final bool verbose; final bool printFailureLog; final String configurationName; - final String testFilter; + final String? testFilter; final List environmentOptions; + final int shard; const SuiteConfiguration( - this.name, - this.resultsPort, - this.logsPort, - this.verbose, - this.printFailureLog, - this.configurationName, - this.testFilter, - this.environmentOptions); + this.suite, + this.resultsPort, + this.logsPort, + this.verbose, + this.printFailureLog, + this.configurationName, + this.testFilter, + this.environmentOptions, + this.shard, + ); } -void runSuite(SuiteConfiguration configuration) { - Suite suite = suites.where((s) => s.name == configuration.name).single; +void runSuite(SuiteConfiguration configuration) async { + Suite suite = configuration.suite; String name = suite.prefix; String fullSuiteName = "$suiteNamePrefix/$name"; Uri suiteUri = Platform.script.resolve(suite.path ?? "${name}_suite.dart"); @@ -334,23 +471,30 @@ void runSuite(SuiteConfiguration configuration) { configuration.verbose, configuration.printFailureLog, configuration.configurationName); - runMe([ - if (configuration.testFilter != null) configuration.testFilter, - if (configuration.environmentOptions != null) + await runMe( + [ + if (configuration.testFilter != null) configuration.testFilter!, for (String option in configuration.environmentOptions) '-D${option}', - ], suite.createContext, - me: suiteUri, - configurationPath: suite.testingRootPath, - logger: logger, - shards: suite.shardCount, - shard: suite.shard); + ], + suite.createContext, + me: suiteUri, + configurationPath: suite.testingRootPath, + logger: logger, + shards: suite.shardCount, + shard: configuration.shard, + ); + if (logger.gotFrameworkError) { + throw "Got framework error!"; + } } -void writeLinesToFile(Uri uri, List lines) async { +Future writeLinesToFile(Uri uri, List lines) async { await File.fromUri(uri).writeAsString(lines.map((line) => "$line\n").join()); } main([List arguments = const []]) async { + Stopwatch totalRuntime = new Stopwatch()..start(); + List results = []; List logs = []; Options options = Options.parse(arguments); @@ -359,13 +503,23 @@ main([List arguments = const []]) async { ReceivePort logsPort = new ReceivePort() ..listen((logEntry) => logs.add(logEntry)); List> futures = []; + + if (options.verbose) { + print("NOTE: Willing to run with ${options.numberOfWorkers} 'workers'"); + print(""); + } + + int numberOfFreeWorkers = options.numberOfWorkers; // Run test suites and record the results and possible failure logs. + int chunkNum = 0; for (Suite suite in suites) { - String name = suite.name; - String filter = options.testFilter; + if (options.onlyTestsThatRequireGit && !suite.requiresGit) continue; + if (options.skipTestsThatRequireGit && suite.requiresGit) continue; + String prefix = suite.prefix; + String? filter = options.testFilter; if (filter != null) { // Skip suites that are not hit by the test filter, is there is one. - if (!filter.startsWith(suite.prefix)) { + if (!filter.startsWith(prefix)) { continue; } // Remove the 'fasta/' from filters, if there, because it is not used @@ -374,39 +528,63 @@ main([List arguments = const []]) async { filter = filter.substring("fasta/".length); } } - // Start the test suite in a new isolate. - ReceivePort exitPort = new ReceivePort(); - SuiteConfiguration configuration = SuiteConfiguration( - name, - resultsPort.sendPort, - logsPort.sendPort, - options.verbose, - options.printFailureLog, - options.configurationName, - filter, - options.environmentOptions); - Future future = Future(() async { - Stopwatch stopwatch = Stopwatch()..start(); - print("Running suite $name"); - Isolate isolate = await Isolate.spawn( - runSuite, configuration, - onExit: exitPort.sendPort); - bool timedOut = false; - Timer timer = Timer(timeoutDuration, () { - timedOut = true; - print("Suite $name timed out after " - "${timeoutDuration.inMilliseconds}ms"); - isolate.kill(priority: Isolate.immediate); - }); - await exitPort.first; - timer.cancel(); - if (!timedOut) { - int seconds = stopwatch.elapsedMilliseconds ~/ 1000; - print("Suite $name finished (took ${seconds} seconds)"); + for (int shard = 0; shard < suite.shardCount; shard++) { + if (chunkNum++ % options.shardCount != options.shard) continue; + + while (numberOfFreeWorkers <= 0) { + // This might not be great design, but it'll work fine. + await Future.delayed(const Duration(milliseconds: 50)); } - return timedOut; - }); - futures.add(future); + numberOfFreeWorkers--; + // Start the test suite in a new isolate. + ReceivePort exitPort = new ReceivePort(); + ReceivePort errorPort = new ReceivePort(); + SuiteConfiguration configuration = new SuiteConfiguration( + suite, + resultsPort.sendPort, + logsPort.sendPort, + options.verbose, + options.printFailureLog, + options.configurationName, + filter, + options.environmentOptions, + shard); + Future future = new Future(() async { + try { + Stopwatch stopwatch = new Stopwatch()..start(); + String naming = "$prefix"; + if (suite.shardCount > 1) { + naming += " (${shard + 1} of ${suite.shardCount})"; + } + print("Running suite $naming"); + Isolate isolate = await Isolate.spawn( + runSuite, configuration, + onExit: exitPort.sendPort, onError: errorPort.sendPort); + bool timedOutOrCrash = false; + Timer timer = new Timer(timeoutDuration, () { + timedOutOrCrash = true; + print("Suite $naming timed out after " + "${timeoutDuration.inMilliseconds}ms"); + isolate.kill(priority: Isolate.immediate); + }); + await exitPort.first; + errorPort.close(); + bool gotError = !await errorPort.isEmpty; + if (gotError) { + timedOutOrCrash = true; + } + timer.cancel(); + if (!timedOutOrCrash) { + int seconds = stopwatch.elapsedMilliseconds ~/ 1000; + print("Suite $naming finished (took ${seconds} seconds)"); + } + return timedOutOrCrash; + } finally { + numberOfFreeWorkers++; + } + }); + futures.add(future); + } } // Wait for isolates to terminate and clean up. Iterable timeouts = await Future.wait(futures); @@ -419,6 +597,7 @@ main([List arguments = const []]) async { await writeLinesToFile(logsJsonUri, logs); print("Log files written to ${resultJsonUri.toFilePath()} and" " ${logsJsonUri.toFilePath()}"); + print("Entire run took ${totalRuntime.elapsed}."); // Return with exit code 1 if at least one suite timed out. bool timeout = timeouts.any((timeout) => timeout); if (timeout) { diff --git a/pkg/front_end/test/utils/io_utils.dart b/pkg/front_end/test/utils/io_utils.dart index 6cd9daaaca6..05be4971134 100644 --- a/pkg/front_end/test/utils/io_utils.dart +++ b/pkg/front_end/test/utils/io_utils.dart @@ -11,7 +11,16 @@ String computeRepoDir() { 'git', ['rev-parse', '--show-toplevel'], runInShell: true, workingDirectory: new File.fromUri(Platform.script).parent.path); - return (result.stdout as String).trim(); + if (result.exitCode != 0) { + throw "Git returned non-zero error code (${result.exitCode}):\n\n" + "stdout: ${result.stdout}\n\n" + "stderr: ${result.stderr}"; + } + String dirPath = (result.stdout as String).trim(); + if (!new Directory(dirPath).existsSync()) { + throw "The path returned by git ($dirPath) does not actually exist."; + } + return dirPath; } Uri computeRepoDirUri() { diff --git a/pkg/testing/lib/src/error_handling.dart b/pkg/testing/lib/src/error_handling.dart index bf13cb05b86..452e4b72f6c 100644 --- a/pkg/testing/lib/src/error_handling.dart +++ b/pkg/testing/lib/src/error_handling.dart @@ -10,7 +10,9 @@ import 'dart:io' show exitCode, stderr; import 'dart:isolate' show ReceivePort; -Future withErrorHandling(Future f()) async { +import 'log.dart'; + +Future withErrorHandling(Future f(), {Logger logger}) async { final ReceivePort port = new ReceivePort(); try { return await f(); @@ -20,6 +22,7 @@ Future withErrorHandling(Future f()) async { if (trace != null) { stderr.writeln(trace); } + logger?.noticeFrameworkCatchError(e, trace); return null; } finally { port.close(); diff --git a/pkg/testing/lib/src/log.dart b/pkg/testing/lib/src/log.dart index e916ba97970..7e6fafd0f71 100644 --- a/pkg/testing/lib/src/log.dart +++ b/pkg/testing/lib/src/log.dart @@ -68,6 +68,11 @@ abstract class Logger { void logSuiteComplete(Suite suite); void logUncaughtError(error, StackTrace stackTrace); + + /// Issued when there's been a crash caught by the framework. + /// Notice that the exit-code has already been set and that the error has + /// been printed to stderr. + void noticeFrameworkCatchError(error, StackTrace stackTrace); } class StdoutLogger implements Logger { @@ -185,6 +190,8 @@ class StdoutLogger implements Logger { logMessage(stackTrace); } } + + void noticeFrameworkCatchError(error, StackTrace stackTrace) {} } String pad(Object o, int pad, {String filler: " "}) { diff --git a/pkg/testing/lib/src/run.dart b/pkg/testing/lib/src/run.dart index 1930c457d49..7ce9bd02dbf 100644 --- a/pkg/testing/lib/src/run.dart +++ b/pkg/testing/lib/src/run.dart @@ -67,7 +67,7 @@ Future runMe(List arguments, CreateContext f, shards: shards, shard: shard, logger: logger); } } - }); + }, logger: logger); } /// This is called from a `_test.dart` file, and helps integration in other diff --git a/tools/bots/test_matrix.json b/tools/bots/test_matrix.json index 12f746b65c6..29f0ba2ace0 100644 --- a/tools/bots/test_matrix.json +++ b/tools/bots/test_matrix.json @@ -964,7 +964,7 @@ "co19_2" ], "fileset": "front-end", - "shards": 10 + "shards": 2 }, { "name": "sdk tests", @@ -972,22 +972,44 @@ "-ncfe-${system}" ], "fileset": "front-end", - "shards": 5 + "shards": 2 }, { - "name": "unit tests", + "name": "unit tests (no git)", "arguments": [ "-ncfe-unittest-asserts-${mode}-${system}", - "pkg/pkg/(kernel|front_end|fasta)/" - ] + "pkg/pkg/(kernel|front_end|fasta)\/(*?)(?