From 9c0a6fd27c0f60dc4265765683f7e4c5eb2358be Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Fri, 20 Oct 2017 16:00:28 +0000 Subject: [PATCH] Add rudimentary support for dartdevk to test.dart. It's added as a new compiler, so pass "-c dartdevk" to use it. It doesn't support any test packages yet, so tests that, say, import package expect won't compile. I'll work on that next, but it will require adding some stuff to the build scripts to build .dill files for those packages. This does get test.dart invoking the compiler, running the resulting test, and correctly reporting the result: - A test that doesn't throw an exception and stays within the bounds of what is currently implemented in dartdevk passes. - A test that compiles correctly but fails at runtime fails with a RuntimeError. - A test that contains a compile error fails with a non-zero exit code and is reported as a CompileTimeError. Change-Id: Icacbf1ff54dfe7aa4d245382d3b0aeb375cf105b Reviewed-on: https://dart-review.googlesource.com/15420 Commit-Queue: Bob Nystrom Reviewed-by: Vijay Menon --- pkg/dev_compiler/bin/dartdevk.dart | 39 +++++++++++- pkg/dev_compiler/lib/src/kernel/command.dart | 39 ++++++++---- pkg/dev_compiler/lib/src/kernel/compiler.dart | 9 +-- tools/testing/dart/command.dart | 12 ++-- tools/testing/dart/command_output.dart | 1 - .../testing/dart/compiler_configuration.dart | 62 +++++++++++++++++++ tools/testing/dart/configuration.dart | 3 + tools/testing/dart/test_controller.js | 2 +- tools/testing/dart/test_suite.dart | 8 ++- 9 files changed, 150 insertions(+), 25 deletions(-) diff --git a/pkg/dev_compiler/bin/dartdevk.dart b/pkg/dev_compiler/bin/dartdevk.dart index dde27b2b54d..50d5387a9cd 100755 --- a/pkg/dev_compiler/bin/dartdevk.dart +++ b/pkg/dev_compiler/bin/dartdevk.dart @@ -5,7 +5,44 @@ /// Experimental command line entry point for Dart Development Compiler. /// Unlike `dartdevc` this version uses the shared front end and IR. +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; import 'package:dev_compiler/src/kernel/command.dart'; -main(List args) => compile(args); +Future main(List args) async { + if (args.isNotEmpty && args.last == "--batch") { + await runBatch(args.sublist(0, args.length - 1)); + } else { + var succeeded = await compile(args); + exitCode = succeeded ? 0 : 1; + } +} + +/// Runs dartdevk in batch mode for test.dart. +Future runBatch(List batchArgs) async { + var tests = 0; + var failed = 0; + var watch = new Stopwatch()..start(); + + print('>>> BATCH START'); + + String line; + while ((line = stdin.readLineSync(encoding: UTF8)).isNotEmpty) { + tests++; + var args = batchArgs.toList()..addAll(line.split(new RegExp(r'\s+'))); + + var succeeded = await compile(args); + + // TODO(rnystrom): If kernel has any internal static state that needs to + // be cleared, do it here. + + stderr.writeln('>>> EOF STDERR'); + var outcome = succeeded ? 'PASS' : 'FAIL'; + print('>>> TEST $outcome ${watch.elapsedMilliseconds}ms'); + } + + var time = watch.elapsedMilliseconds; + print('>>> BATCH END (${tests - failed})/$tests ${time}ms'); +} diff --git a/pkg/dev_compiler/lib/src/kernel/command.dart b/pkg/dev_compiler/lib/src/kernel/command.dart index 507ed85caa4..6a63d28e806 100644 --- a/pkg/dev_compiler/lib/src/kernel/command.dart +++ b/pkg/dev_compiler/lib/src/kernel/command.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'dart:io'; import 'package:args/args.dart'; import 'package:dev_compiler/src/kernel/target.dart'; +import 'package:front_end/compilation_message.dart'; import 'package:front_end/compiler_options.dart'; import 'package:front_end/kernel_generator.dart'; import 'package:kernel/kernel.dart'; @@ -18,30 +19,45 @@ import '../js_ast/js_ast.dart' as JS; import 'compiler.dart'; import 'native_types.dart'; -Future compile(List args) async { +/// Invoke the compiler with [args]. +/// +/// Returns `true` if the program compiled without any fatal errors. +Future compile(List args) async { var ddcPath = path.dirname(path.dirname(path.fromUri(Platform.script))); var argResults = (new ArgParser(allowTrailingOptions: true) ..addOption('out', abbr: 'o', help: 'Output file (required).')) .parse(args); + + var succeeded = true; + + void errorHandler(CompilationMessage error) { + if (error.severity == Severity.error) succeeded = false; + } + var options = new CompilerOptions() ..sdkSummary = path.toUri(path.absolute(ddcPath, 'lib', 'sdk', 'ddc_sdk.dill')) ..packagesFileUri = path.toUri(path.absolute(ddcPath, '..', '..', '.packages')) - ..throwOnErrorsForDebugging = true - ..target = new DevCompilerTarget(); + ..target = new DevCompilerTarget() + ..onError = errorHandler + ..reportMessages = true; var inputs = argResults.rest.map(path.toUri).toList(); var output = argResults['out']; var program = await kernelForBuildUnit(inputs, options); - // Useful for debugging: - writeProgramToText(program); - // TODO(jmesserly): save .dill file so other modules can link in this one. - //await writeProgramToBinary(program, output); - var jsCode = compileToJSModule(program); - new File(output).writeAsStringSync(jsCode); + if (succeeded) { + // Useful for debugging: + writeProgramToText(program); + // TODO(jmesserly): Save .dill file so other modules can link in this one. + //await writeProgramToBinary(program, output); + var jsCode = compileToJSModule(program); + new File(output).writeAsStringSync(jsCode); + } + + return succeeded; } String compileToJSModule(Program p) { @@ -53,10 +69,11 @@ String compileToJSModule(Program p) { String jsProgramToString(JS.Program moduleTree) { var opts = new JS.JavaScriptPrintingOptions( allowKeywordsInProperties: true, allowSingleLineIfStatements: true); - // TODO(jmesserly): support source maps + // TODO(jmesserly): Support source maps. var printer = new JS.SimpleJavaScriptPrintingContext(); - var tree = transformModuleFormat(ModuleFormat.common, moduleTree); + // TODO(rnystrom): Allow specifying other module formats. + var tree = transformModuleFormat(ModuleFormat.amd, moduleTree); tree.accept( new JS.Printer(opts, printer, localNamer: new JS.TemporaryNamer(tree))); diff --git a/pkg/dev_compiler/lib/src/kernel/compiler.dart b/pkg/dev_compiler/lib/src/kernel/compiler.dart index 6e5c51bf084..cd27182fb75 100644 --- a/pkg/dev_compiler/lib/src/kernel/compiler.dart +++ b/pkg/dev_compiler/lib/src/kernel/compiler.dart @@ -1194,16 +1194,17 @@ String jsLibraryName(Library library) { if (uri.scheme == 'dart') return uri.path; // TODO(vsm): This is not necessarily unique if '__' appears in a file name. - Iterable segements; + Iterable segments; if (uri.scheme == 'package') { // Strip the package name. // TODO(vsm): This is not unique if an escaped '/'appears in a filename. // E.g., "foo/bar.dart" and "foo__bar.dart" would collide. - segements = uri.pathSegments.skip(1); + segments = uri.pathSegments.skip(1); } else { - segements = path.split(path.relative(uri.toFilePath())); + segments = path.split(path.relative(uri.toFilePath())); } - var qualifiedPath = segements.map((p) => p == '..' ? '' : p).join('__'); + + var qualifiedPath = segments.map((p) => p == '..' ? '' : p).join('__'); return pathToJSIdentifier(qualifiedPath); } diff --git a/tools/testing/dart/command.dart b/tools/testing/dart/command.dart index 5a49bc1706c..835de90e354 100644 --- a/tools/testing/dart/command.dart +++ b/tools/testing/dart/command.dart @@ -45,9 +45,11 @@ class Command { String executable, List arguments, Map environment, - {bool alwaysCompile: false}) { + {bool alwaysCompile: false, + String workingDirectory}) { return new CompilationCommand._(displayName, outputFile, alwaysCompile, - bootstrapDependencies, executable, arguments, environment); + bootstrapDependencies, executable, arguments, environment, + workingDirectory: workingDirectory); } static Command kernelCompilation( @@ -230,8 +232,10 @@ class CompilationCommand extends ProcessCommand { this._bootstrapDependencies, String executable, List arguments, - Map environmentOverrides) - : super._(displayName, executable, arguments, environmentOverrides); + Map environmentOverrides, + {String workingDirectory}) + : super._(displayName, executable, arguments, environmentOverrides, + workingDirectory); bool get outputIsUpToDate { if (_alwaysCompile) return false; diff --git a/tools/testing/dart/command_output.dart b/tools/testing/dart/command_output.dart index d340e29bc9b..4079b21326f 100644 --- a/tools/testing/dart/command_output.dart +++ b/tools/testing/dart/command_output.dart @@ -12,7 +12,6 @@ import 'package:status_file/expectation.dart'; import 'browser_controller.dart'; import 'command.dart'; import 'configuration.dart'; -import 'test_progress.dart'; import 'test_runner.dart'; import 'utils.dart'; diff --git a/tools/testing/dart/compiler_configuration.dart b/tools/testing/dart/compiler_configuration.dart index 6e3c9d4d274..48b24b9f681 100644 --- a/tools/testing/dart/compiler_configuration.dart +++ b/tools/testing/dart/compiler_configuration.dart @@ -57,6 +57,9 @@ abstract class CompilerConfiguration { case Compiler.dartdevc: return new DevCompilerConfiguration(configuration); + case Compiler.dartdevk: + return new DevKernelCompilerConfiguration(configuration); + case Compiler.appJit: return new AppJitCompilerConfiguration(configuration); @@ -451,6 +454,65 @@ class DevCompilerConfiguration extends CompilerConfiguration { } } +/// Configuration for dev-compiler with the kernel front end. +class DevKernelCompilerConfiguration extends CompilerConfiguration { + DevKernelCompilerConfiguration(Configuration configuration) + : super._subclass(configuration); + + String computeCompilerPath() => "pkg/dev_compiler/bin/dartdevk.dart"; + + List computeCompilerArguments( + List vmOptions, List sharedOptions, List args) { + var result = sharedOptions.toList(); + + // The file being compiled is the last argument. + result.add(args.last); + return result; + } + + Command createCommand( + String inputFile, String outputFile, List sharedOptions, + [Map environment = const {}]) { + var args = sharedOptions.toList(); + args.addAll([ + "-o", + outputFile, + inputFile, + ]); + + // TODO(rnystrom): Link to dill files for the packages used by tests. + + // Use the directory containing the test as the working directory. This + // ensures dartdevk creates a short module named based on the test name + // (like "ackermann_test") and does not include any of the parent + // directories in the name (like "tests__language_2__ackermann_test"). + var inputDir = + new Path(inputFile).append("..").canonicalize().toNativePath(); + var compiler = Repository.dir.append(computeCompilerPath()).toNativePath(); + + return Command.compilation(Compiler.dartdevk.name, outputFile, + bootstrapDependencies(), compiler, args, environment, + workingDirectory: inputDir); + } + + CommandArtifact computeCompilationArtifact( + String tempDir, List arguments, Map environment) { + // The list of arguments comes from a call to our own + // computeCompilerArguments(). It contains the shared options followed by + // the input file path. + // TODO(rnystrom): Jamming these into a list in order to pipe them from + // computeCompilerArguments() to here seems hacky. Is there a cleaner way? + var sharedOptions = arguments.sublist(0, arguments.length - 1); + var inputFile = arguments.last; + var outputFile = "$tempDir/${inputFile.replaceAll('.dart', '.js')}"; + + return new CommandArtifact( + [createCommand(inputFile, outputFile, sharedOptions, environment)], + outputFile, + "application/javascript"); + } +} + class PrecompilerCompilerConfiguration extends CompilerConfiguration { final bool useDfe; diff --git a/tools/testing/dart/configuration.dart b/tools/testing/dart/configuration.dart index 9bbf18071f9..c85141487da 100644 --- a/tools/testing/dart/configuration.dart +++ b/tools/testing/dart/configuration.dart @@ -517,6 +517,7 @@ class Compiler { static const dart2js = const Compiler._('dart2js'); static const dart2analyzer = const Compiler._('dart2analyzer'); static const dartdevc = const Compiler._('dartdevc'); + static const dartdevk = const Compiler._('dartdevk'); static const appJit = const Compiler._('app_jit'); static const dartk = const Compiler._('dartk'); static const dartkp = const Compiler._('dartkp'); @@ -530,6 +531,7 @@ class Compiler { dart2js, dart2analyzer, dartdevc, + dartdevk, appJit, dartk, dartkp, @@ -573,6 +575,7 @@ class Compiler { case Compiler.dart2js: case Compiler.dartdevc: + case Compiler.dartdevk: // TODO(rnystrom): Expand to support other JS execution environments // (other browsers, d8) when tested and working. return const [ diff --git a/tools/testing/dart/test_controller.js b/tools/testing/dart/test_controller.js index 80153727e49..21ab38ea6e4 100644 --- a/tools/testing/dart/test_controller.js +++ b/tools/testing/dart/test_controller.js @@ -328,7 +328,7 @@ function dartPrint(message) { // dart2js will generate code to call this function instead of calling // Dart [main] directly. The argument is a closure that invokes main. -function dartMainRunner(main, getStackTrace) { +function dartMainRunner(main) { dartPrint('dart-calling-main'); try { main(); diff --git a/tools/testing/dart/test_suite.dart b/tools/testing/dart/test_suite.dart index 6c3c8b0ceca..bd90d90dde7 100644 --- a/tools/testing/dart/test_suite.dart +++ b/tools/testing/dart/test_suite.dart @@ -1036,7 +1036,7 @@ class StandardTestSuite extends TestSuite { // Synthesize an HTML file for the test. var scriptPath = _createUrlPathFromFile(new Path(jsWrapperFileName)); - if (configuration.compiler != Compiler.dartdevc) { + if (configuration.compiler == Compiler.dart2js) { content = dart2jsHtml(fileName, scriptPath); } else { var jsDir = @@ -1059,6 +1059,7 @@ class StandardTestSuite extends TestSuite { break; case Compiler.dartdevc: + case Compiler.dartdevk: var toPath = new Path('$compilationTempDir/$nameNoExt.js').toNativePath(); commands.add(configuration.compilerConfiguration.createCommand(fileName, @@ -1082,6 +1083,7 @@ class StandardTestSuite extends TestSuite { break; case Compiler.dartdevc: + case Compiler.dartdevk: commands.add(configuration.compilerConfiguration.createCommand( fromPath.toNativePath(), toPath, @@ -1154,9 +1156,9 @@ class StandardTestSuite extends TestSuite { var compiler = configuration.compiler; var runtime = configuration.runtime; - if (compiler == Compiler.dartdevc) { + if (compiler == Compiler.dartdevc || compiler == Compiler.dartdevk) { // TODO(rnystrom): Support this for dartdevc (#29919). - print("Ignoring $testName on dartdevc since HTML tests are not " + print("Ignoring $testName on ${compiler.name} since HTML tests are not " "implemented for that compiler yet."); return; }