From 0b2c9b048d613bd81caf07acd285f61cd01b79dd Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Thu, 29 Jun 2017 13:45:55 -0700 Subject: [PATCH] Dynamically load packages for dartdevc tests in test.dart. This involves a few pieces: - Add support to DDC for specifying the module name associated with a given summary. This lets test.dart invoke DDC using summaries in the build directory outside of the directory containing the test itself. - Add support to the build scripts for building the packages. This adds a new GN target that builds everything needed to run test.dart with dartdevc. In particular, it invokes build_pkgs.dart to compile the relevant packages to JS+summary so that the tests can use them. This requires some changes to build_pkgs.dart so it can output to a given directory. - In test.dart, when tests are compiled with dartdevc, pass in the summaries for the packages so they don't get compiled in. Then, when the test is run, configure require.js with the right paths to their JS files so they can be loaded. I also removed a bunch of unneeded buildDir parameters being passed around the various CompilerConfiguration class methods now that they have direct access to the configuration. Fix #29923. R=vsm@google.com, whesse@google.com, zra@google.com Review-Url: https://codereview.chromium.org/2955513002 . --- .travis.yml | 2 +- .../lib/src/analyzer/context.dart | 21 ++ .../lib/src/compiler/command.dart | 17 +- .../lib/src/compiler/compiler.dart | 10 +- .../test/options/options_test.dart | 17 ++ pkg/dev_compiler/tool/build_pkgs.dart | 55 +++-- pkg/dev_compiler/tool/build_pkgs.sh | 2 +- pkg/dev_compiler/tool/build_test_pkgs.sh | 5 - pkg/dev_compiler/tool/test.sh | 2 +- tools/testing/dart/browser_test.dart | 27 ++- .../testing/dart/compiler_configuration.dart | 191 +++++++++--------- tools/testing/dart/test_suite.dart | 42 +--- tools/testing/dart/utils.dart | 13 ++ utils/dartdevc/BUILD.gn | 99 +++++++-- 14 files changed, 317 insertions(+), 186 deletions(-) delete mode 100755 pkg/dev_compiler/tool/build_test_pkgs.sh diff --git a/.travis.yml b/.travis.yml index 6dc20a42a6a..efe6ae53eda 100644 --- a/.travis.yml +++ b/.travis.yml @@ -73,7 +73,7 @@ script: - if [[ -z "$TEST" ]]; then ./tool/presubmit.sh ; fi - if [[ "$TEST" == coverage ]]; then ./tool/build_sdk.sh && ./tool/coverage.sh ; fi - if [[ "$TEST" == node ]]; then ./tool/node_test.sh ; fi - - if [[ "$TEST" == package ]]; then ./tool/build_sdk.sh && ./tool/build_pkgs.dart ; fi + - if [[ "$TEST" == package ]]; then ./tool/build_sdk.sh && ./tool/build_pkgs.dart gen/codegen_output/pkg travis; fi env: - ANALYZER=master - ANALYZER=master DDC_BROWSERS=Firefox diff --git a/pkg/dev_compiler/lib/src/analyzer/context.dart b/pkg/dev_compiler/lib/src/analyzer/context.dart index 188f9787624..496ae800fdf 100644 --- a/pkg/dev_compiler/lib/src/analyzer/context.dart +++ b/pkg/dev_compiler/lib/src/analyzer/context.dart @@ -32,6 +32,8 @@ class AnalyzerOptions { /// List of summary file paths. final List summaryPaths; + final Map customSummaryModules = {}; + /// Path to the dart-sdk, or `null` if the path couldn't be determined. final String dartSdkPath; @@ -51,6 +53,7 @@ class AnalyzerOptions { : dartSdkPath = dartSdkPath ?? getSdkDir().path, summaryPaths = summaryPaths ?? const [] { contextBuilderOptions.declaredVariables ??= const {}; + _parseCustomSummaryModules(); } factory AnalyzerOptions.basic( @@ -109,6 +112,24 @@ class AnalyzerOptions { } return mappings; } + + /// A summary path can contain ":" followed by an explicit module name to + /// allow working with summaries whose physical location is outside of the + /// module root directory. + /// + /// Removes any explicit module names from [summaryPaths] and populates with + /// [customSummaryModules] with them. + void _parseCustomSummaryModules() { + for (var i = 0; i < summaryPaths.length; i++) { + var summaryPath = summaryPaths[i]; + var comma = summaryPath.indexOf(":"); + if (comma != -1) { + summaryPaths[i] = summaryPath.substring(0, comma); + customSummaryModules[summaryPaths[i]] = + summaryPath.substring(comma + 1); + } + } + } } /// Creates a SourceFactory configured by the [options]. diff --git a/pkg/dev_compiler/lib/src/compiler/command.dart b/pkg/dev_compiler/lib/src/compiler/command.dart index af34e95f711..dfce39705c4 100644 --- a/pkg/dev_compiler/lib/src/compiler/command.dart +++ b/pkg/dev_compiler/lib/src/compiler/command.dart @@ -180,8 +180,12 @@ void _compile(ArgResults argResults, AnalyzerOptions analyzerOptions, modulePath = path.basenameWithoutExtension(firstOutPath); } - var unit = new BuildUnit(modulePath, libraryRoot, argResults.rest, - (source) => _moduleForLibrary(moduleRoot, source, compilerOpts)); + var unit = new BuildUnit( + modulePath, + libraryRoot, + argResults.rest, + (source) => + _moduleForLibrary(moduleRoot, source, analyzerOptions, compilerOpts)); var module = compiler.compile(unit, compilerOpts); module.errors.forEach(printFn); @@ -217,10 +221,15 @@ void _compile(ArgResults argResults, AnalyzerOptions analyzerOptions, } } -String _moduleForLibrary( - String moduleRoot, Source source, CompilerOptions compilerOpts) { +String _moduleForLibrary(String moduleRoot, Source source, + AnalyzerOptions analyzerOptions, CompilerOptions compilerOpts) { if (source is InSummarySource) { var summaryPath = source.summaryPath; + + if (analyzerOptions.customSummaryModules.containsKey(summaryPath)) { + return analyzerOptions.customSummaryModules[summaryPath]; + } + var ext = '.${compilerOpts.summaryExtension}'; if (path.isWithin(moduleRoot, summaryPath) && summaryPath.endsWith(ext)) { var buildUnitPath = diff --git a/pkg/dev_compiler/lib/src/compiler/compiler.dart b/pkg/dev_compiler/lib/src/compiler/compiler.dart index ac1d60a6e66..73f0914dfbf 100644 --- a/pkg/dev_compiler/lib/src/compiler/compiler.dart +++ b/pkg/dev_compiler/lib/src/compiler/compiler.dart @@ -254,7 +254,7 @@ class CompilerOptions { /// The file extension for summaries. final String summaryExtension; - /// Whether to preserve metdata only accessible via mirrors + /// Whether to preserve metdata only accessible via mirrors. final bool emitMetadata; /// Whether to force compilation of code with static errors. @@ -267,16 +267,16 @@ class CompilerOptions { /// Whether to emit Closure Compiler-friendly code. final bool closure; - /// Hoist the types at instance creation sites + /// Hoist the types at instance creation sites. final bool hoistInstanceCreation; - /// Hoist types from class signatures + /// Hoist types from class signatures. final bool hoistSignatureTypes; - /// Name types in type tests + /// Name types in type tests. final bool nameTypeTests; - /// Hoist types in type tests + /// Hoist types in type tests. final bool hoistTypeTests; /// Enable ES6 destructuring of named parameters. Off by default. diff --git a/pkg/dev_compiler/test/options/options_test.dart b/pkg/dev_compiler/test/options/options_test.dart index 5fff692a8a1..a87034d5969 100644 --- a/pkg/dev_compiler/test/options/options_test.dart +++ b/pkg/dev_compiler/test/options/options_test.dart @@ -79,4 +79,21 @@ main() { var analysisOptions = compiler.context.analysisOptions; expect(analysisOptions.enableStrictCallChecks, isTrue); }); + + test('custom module name for summary', () { + var args = [ + '-snormal', + '-scustom/path:module', + '-sanother', + '-scustom/path2:module2' + ]; + + var argResults = ddcArgParser().parse(args); + var options = new AnalyzerOptions.fromArguments(argResults); + expect(options.summaryPaths, + orderedEquals(['normal', 'custom/path', 'another', 'custom/path2'])); + expect(options.customSummaryModules['custom/path'], equals('module')); + expect(options.customSummaryModules['custom/path2'], equals('module2')); + expect(options.customSummaryModules.containsKey('normal'), isFalse); + }); } diff --git a/pkg/dev_compiler/tool/build_pkgs.dart b/pkg/dev_compiler/tool/build_pkgs.dart index 04be68b647e..df46e3f5cf3 100755 --- a/pkg/dev_compiler/tool/build_pkgs.dart +++ b/pkg/dev_compiler/tool/build_pkgs.dart @@ -1,20 +1,31 @@ #!/usr/bin/env dart import 'dart:io'; +import 'package:path/path.dart' as p; + import 'package:dev_compiler/src/compiler/command.dart'; -/// Compiles the packages that the DDC tests use to JS into: -/// -/// gen/codegen_output/pkg/... -/// -/// Assumes the working directory is pkg/dev_compiler. -/// -/// If no arguments are passed, builds the all of the modules tested on Travis. -/// If "test" is passed, only builds the modules needed by the tests. -void main(List arguments) { - var test = arguments.length == 1 && arguments[0] == 'test'; +final String scriptDirectory = p.dirname(p.fromUri(Platform.script)); +String outputDirectory; - new Directory("gen/codegen_output/pkg").createSync(recursive: true); +/// Compiles the packages that the DDC tests use to JS into the given output +/// directory. Usage: +/// +/// dart build_pkgs.dart [travis] +/// +/// If "travis" is passed, builds the all of the modules tested on Travis. +/// Otherwise, only builds the modules needed by the tests. +void main(List arguments) { + var isTravis = arguments.isNotEmpty && arguments.last == "travis"; + if (isTravis) arguments.removeLast(); + + if (arguments.length != 1) { + print("Usage: dart build_pkgs.dart [travis]"); + exit(1); + } + + outputDirectory = arguments[0]; + new Directory(outputDirectory).createSync(recursive: true); // Build leaf packages. These have no other package dependencies. @@ -23,7 +34,7 @@ void main(List arguments) { compileModule('expect', libs: ['minitest']); compileModule('js', libs: ['js_util']); compileModule('meta'); - if (!test) { + if (isTravis) { compileModule('lookup_map'); compileModule('microlytics', libs: ['html_channels']); compileModule('typed_mock'); @@ -33,7 +44,7 @@ void main(List arguments) { compileModule('collection'); compileModule('matcher'); compileModule('path'); - if (!test) { + if (isTravis) { compileModule('args', libs: ['command_runner']); compileModule('charcode'); compileModule('fixnum'); @@ -49,11 +60,11 @@ void main(List arguments) { // Composite packages with dependencies. compileModule('stack_trace', deps: ['path']); - if (!test) { + if (isTravis) { compileModule('async', deps: ['collection']); } - if (test) { + if (!isTravis) { compileModule('unittest', deps: ['matcher', 'path', 'stack_trace'], libs: ['html_config', 'html_individual_config', 'html_enhanced_config'], @@ -65,9 +76,10 @@ void main(List arguments) { /// [libs] and [deps] on other modules. void compileModule(String module, {List libs, List deps, bool unsafeForceCompile: false}) { + var sdkSummary = p.join(scriptDirectory, "../lib/sdk/ddc_sdk.sum"); var args = [ - '--dart-sdk-summary=lib/sdk/ddc_sdk.sum', - '-ogen/codegen_output/pkg/$module.js' + '--dart-sdk-summary=$sdkSummary', + '-o${outputDirectory}/$module.js' ]; // There is always a library that matches the module. @@ -83,7 +95,7 @@ void compileModule(String module, // Add summaries for any modules this depends on. if (deps != null) { for (var dep in deps) { - args.add('-sgen/codegen_output/pkg/$dep.sum'); + args.add('-s${outputDirectory}/$dep.sum'); } } @@ -96,9 +108,10 @@ void compileModule(String module, // but I'm not sure how they'll affect the other non-DDC tests. For now, just // use ours. if (module == 'async_helper') { - args.add('--url-mapping=package:async_helper/async_helper.dart,' - 'test/codegen/async_helper.dart'); + args.add('--url-mapping=package:async_helper/async_helper.dart,' + + p.join(scriptDirectory, "../test/codegen/async_helper.dart")); } - compile(args); + var exitCode = compile(args); + if (exitCode != 0) exit(exitCode); } diff --git a/pkg/dev_compiler/tool/build_pkgs.sh b/pkg/dev_compiler/tool/build_pkgs.sh index 87ec3c09112..fa71c32999f 100755 --- a/pkg/dev_compiler/tool/build_pkgs.sh +++ b/pkg/dev_compiler/tool/build_pkgs.sh @@ -2,4 +2,4 @@ # TODO: This script is deprecated in favor of the Dart version. For now, forward # to it so existing scripts don't break. Eventually, delete this one. -./tool/build_pkgs.dart +./tool/build_pkgs.dart gen/codegen_output/pkg travis diff --git a/pkg/dev_compiler/tool/build_test_pkgs.sh b/pkg/dev_compiler/tool/build_test_pkgs.sh deleted file mode 100755 index 1aa3ddf51bb..00000000000 --- a/pkg/dev_compiler/tool/build_test_pkgs.sh +++ /dev/null @@ -1,5 +0,0 @@ -#!/bin/bash - -# TODO: This script is deprecated in favor of the Dart version. For now, forward -# to it so existing scripts don't break. Eventually, delete this one. -./tool/build_pkgs.dart test diff --git a/pkg/dev_compiler/tool/test.sh b/pkg/dev_compiler/tool/test.sh index f5593bcedf6..f32a6ad4db8 100755 --- a/pkg/dev_compiler/tool/test.sh +++ b/pkg/dev_compiler/tool/test.sh @@ -27,7 +27,7 @@ if [ -d gen/codegen_output ]; then rm -r gen/codegen_output || fail fi -./tool/build_pkgs.dart test +./tool/build_test_pkgs.sh # Make sure we don't run tests in code coverage mode. # this will cause us to generate files that are not part of the baseline diff --git a/tools/testing/dart/browser_test.dart b/tools/testing/dart/browser_test.dart index 862375d8722..b58a1ba1faa 100644 --- a/tools/testing/dart/browser_test.dart +++ b/tools/testing/dart/browser_test.dart @@ -2,6 +2,8 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'utils.dart'; + String getHtmlContents(String title, String scriptType, String scriptPath) { return """ @@ -38,7 +40,13 @@ String getHtmlContents(String title, String scriptType, String scriptPath) { /// The [testName] is the short name of the test without any subdirectory path /// or extension, like "math_test". The [testJSDir] is the relative path to the /// build directory where the dartdevc-generated JS file is stored. -String dartdevcHtml(String testName, String testJSDir) => """ +String dartdevcHtml(String testName, String testJSDir, String buildDir) { + var packagePaths = testPackages + .map((package) => ' "$package": "/root_dart/$buildDir/gen/utils/' + 'dartdevc/pkg/$package",') + .join("\n"); + + return """ @@ -60,10 +68,9 @@ String dartdevcHtml(String testName, String testJSDir) => """