Factor out a SuiteConfiguration object. (#492)

This tracks configuration that makes sense to apply to test suites, as
opposed to Configuration which applies to an entire run and Metadata
which applies to individual tests. This is intended to make it easier
for load separate configurations for separate targets in a live test
runner.

lib/src/backend/metadata.dart

@@ -19,21 +19,29 @@ import 'test_platform.dart';
 /// This metadata comes from declarations on the test itself; it doesn't include
 /// configuration from the user.
 class Metadata {
+  /// Empty metadata with only default values.
+  ///
+  /// Using this is slightly more efficient than manually constructing a new
+  /// metadata with no arguments.
+  static final empty = new Metadata._();
+
   /// The selector indicating which platforms the suite supports.
   final PlatformSelector testOn;
 
   /// The modification to the timeout for the test or suite.
   final Timeout timeout;
 
   /// Whether the test or suite should be skipped.
-  final bool skip;
-
-  /// Whether to use verbose stack traces.
-  final bool verboseTrace;
+  bool get skip => _skip ?? false;
+  final bool _skip;
 
   /// The reason the test or suite should be skipped, if given.
   final String skipReason;
 
+  /// Whether to use verbose stack traces.
+  bool get verboseTrace => _verboseTrace ?? false;
+  final bool _verboseTrace;
+
   /// The user-defined tags attached to the test or suite.
   final Set<String> tags;
 
@@ -122,8 +130,8 @@ class Metadata {
   /// If [forTag] contains metadata that applies to [tags], that metadata is
   /// included inline in the returned value. The values directly passed to the
   /// constructor take precedence over tag-specific metadata.
-  factory Metadata({PlatformSelector testOn, Timeout timeout, bool skip: false,
-          bool verboseTrace: false, String skipReason, Iterable<String> tags,
+  factory Metadata({PlatformSelector testOn, Timeout timeout, bool skip,
+          bool verboseTrace, String skipReason, Iterable<String> tags,
           Map<PlatformSelector, Metadata> onPlatform,
           Map<BooleanSelector, Metadata> forTag}) {
     // Returns metadata without forTag resolved at all.
@@ -159,13 +167,14 @@ class Metadata {
   /// Creates new Metadata.
   ///
   /// Unlike [new Metadata], this assumes [forTag] is already resolved.
-  Metadata._({PlatformSelector testOn, Timeout timeout, bool skip: false,
-          this.verboseTrace: false, this.skipReason, Iterable<String> tags,
+  Metadata._({PlatformSelector testOn, Timeout timeout, bool skip,
+          this.skipReason, bool verboseTrace, Iterable<String> tags,
           Map<PlatformSelector, Metadata> onPlatform,
           Map<BooleanSelector, Metadata> forTag})
       : testOn = testOn == null ? PlatformSelector.all : testOn,
         timeout = timeout == null ? const Timeout.factor(1) : timeout,
-        skip = skip,
+        _skip = skip,
+        _verboseTrace = verboseTrace,
         tags = new UnmodifiableSetView(
             tags == null ? new Set() : tags.toSet()),
         onPlatform = onPlatform == null
@@ -182,13 +191,14 @@ class Metadata {
   ///
   /// Throws a [FormatException] if any field is invalid.
   Metadata.parse({String testOn, Timeout timeout, skip,
-          this.verboseTrace: false, Map<String, dynamic> onPlatform,
+          bool verboseTrace, Map<String, dynamic> onPlatform,
           tags})
       : testOn = testOn == null
             ? PlatformSelector.all
             : new PlatformSelector.parse(testOn),
         timeout = timeout == null ? const Timeout.factor(1) : timeout,
-        skip = skip != null && skip != false,
+        _skip = skip == null ? null : skip != false,
+        _verboseTrace = verboseTrace,
         skipReason = skip is String ? skip : null,
         onPlatform = _parseOnPlatform(onPlatform),
         tags = _parseTags(tags),
@@ -207,9 +217,9 @@ class Metadata {
             ? PlatformSelector.all
             : new PlatformSelector.parse(serialized['testOn']),
         timeout = _deserializeTimeout(serialized['timeout']),
-        skip = serialized['skip'],
+        _skip = serialized['skip'],
         skipReason = serialized['skipReason'],
-        verboseTrace = serialized['verboseTrace'],
+        _verboseTrace = serialized['verboseTrace'],
         tags = new Set.from(serialized['tags']),
         onPlatform = new Map.fromIterable(serialized['onPlatform'],
             key: (pair) => new PlatformSelector.parse(pair.first),
@@ -252,9 +262,9 @@ class Metadata {
       new Metadata(
           testOn: testOn.intersection(other.testOn),
           timeout: timeout.merge(other.timeout),
-          skip: skip || other.skip,
+          skip: other._skip ?? _skip ,
           skipReason: other.skipReason == null ? skipReason : other.skipReason,
-          verboseTrace: verboseTrace || other.verboseTrace,
+          verboseTrace: other._verboseTrace ?? _verboseTrace,
           tags: tags.union(other.tags),
           onPlatform: mergeMaps(onPlatform, other.onPlatform,
               value: (metadata1, metadata2) => metadata1.merge(metadata2)),
@@ -273,8 +283,8 @@ class Metadata {
       Map<BooleanSelector, Metadata> forTag}) {
     testOn ??= this.testOn;
     timeout ??= this.timeout;
-    skip ??= this.skip;
-    verboseTrace ??= this.verboseTrace;
+    skip ??= this._skip;
+    verboseTrace ??= this._verboseTrace;
     skipReason ??= this.skipReason;
     onPlatform ??= this.onPlatform;
     tags ??= this.tags;

lib/src/runner.dart

@@ -67,20 +67,17 @@ class Runner {
 
   /// Creates a new runner based on [configuration].
   factory Runner(Configuration config) => config.asCurrent(() {
-    var engine = new Engine(
-        concurrency: config.concurrency,
-        runSkipped: config.runSkipped);
+    var engine = new Engine(concurrency: config.concurrency);
 
     var reporter;
     switch (config.reporter) {
       case "expanded":
         reporter = ExpandedReporter.watch(
             engine,
             color: config.color,
-            verboseTrace: config.verboseTrace,
             printPath: config.paths.length > 1 ||
                 new Directory(config.paths.single).existsSync(),
-            printPlatform: config.platforms.length > 1);
+            printPlatform: config.suiteDefaults.platforms.length > 1);
         break;
 
       case "compact":
@@ -124,9 +121,11 @@ class Runner {
 
     if (_closed) return false;
 
-    if (_engine.passed.length == 0 && _engine.failed.length == 0 &&
-        _engine.skipped.length == 0 && _config.patterns.isNotEmpty) {
-      var patterns = toSentence(_config.patterns.map(
+    if (_engine.passed.length == 0 &&
+        _engine.failed.length == 0 &&
+        _engine.skipped.length == 0 &&
+        _config.suiteDefaults.patterns.isNotEmpty) {
+      var patterns = toSentence(_config.suiteDefaults.patterns.map(
           (pattern) => pattern is RegExp
               ? 'regular expression "${pattern.pattern}"'
               : '"$pattern"'));
@@ -142,11 +141,12 @@ class Runner {
   /// Emits a warning if the user is trying to run on a platform that's
   /// unsupported for the entire package.
   void _warnForUnsupportedPlatforms() {
-    if (_config.testOn == PlatformSelector.all) return;
+    var testOn = _config.suiteDefaults.metadata.testOn;
+    if (testOn == PlatformSelector.all) return;
 
-    var unsupportedPlatforms = _config.platforms.where((platform) {
-      return !_config.testOn.evaluate(platform, os: currentOS);
-    }).toList();
+    var unsupportedPlatforms = _config.suiteDefaults.platforms
+        .where((platform) => !testOn.evaluate(platform, os: currentOS))
+        .toList();
     if (unsupportedPlatforms.isEmpty) return;
 
     // Human-readable names for all unsupported platforms.
@@ -160,7 +160,7 @@ class Runner {
     if (unsupportedBrowsers.isNotEmpty) {
       var supportsAnyBrowser = TestPlatform.all
           .where((platform) => platform.isBrowser)
-          .any((platform) => _config.testOn.evaluate(platform));
+          .any((platform) => testOn.evaluate(platform));
 
       if (supportsAnyBrowser) {
         unsupportedNames.addAll(
@@ -174,7 +174,7 @@ class Runner {
     // that's because of the current OS or whether the VM is unsupported.
     if (unsupportedPlatforms.contains(TestPlatform.vm)) {
       var supportsAnyOS = OperatingSystem.all.any((os) =>
-          _config.testOn.evaluate(TestPlatform.vm, os: os));
+          testOn.evaluate(TestPlatform.vm, os: os));
 
       if (supportsAnyOS) {
         unsupportedNames.add(currentOS.name);
@@ -232,29 +232,37 @@ class Runner {
   /// suites once they're loaded.
   Stream<LoadSuite> _loadSuites() {
     return StreamGroup.merge(_config.paths.map((path) {
-      if (new Directory(path).existsSync()) return _loader.loadDir(path);
-      if (new File(path).existsSync()) return _loader.loadFile(path);
-
-      return new Stream.fromIterable([
-        new LoadSuite.forLoadException(
-            new LoadException(path, 'Does not exist.'))
-      ]);
+      if (new Directory(path).existsSync()) {
+        return _loader.loadDir(path, _config.suiteDefaults);
+      } else if (new File(path).existsSync()) {
+        return _loader.loadFile(path, _config.suiteDefaults);
+      } else {
+        return new Stream.fromIterable([
+          new LoadSuite.forLoadException(
+              new LoadException(path, 'Does not exist.'),
+              _config.suiteDefaults)
+        ]);
+      }
     })).map((loadSuite) {
       return loadSuite.changeSuite((suite) {
         _warnForUnknownTags(suite);
 
         return _shardSuite(suite.filter((test) {
           // Skip any tests that don't match all the given patterns.
-          if (!_config.patterns
+          if (!suite.config.patterns
               .every((pattern) => test.name.contains(pattern))) {
             return false;
           }
 
           // If the user provided tags, skip tests that don't match all of them.
-          if (!_config.includeTags.evaluate(test.metadata.tags)) return false;
+          if (!suite.config.includeTags.evaluate(test.metadata.tags)) {
+            return false;
+          }
 
           // Skip tests that do match any tags the user wants to exclude.
-          if (_config.excludeTags.evaluate(test.metadata.tags)) return false;
+          if (suite.config.excludeTags.evaluate(test.metadata.tags)) {
+            return false;
+          }
 
           return true;
         }));
@@ -363,7 +371,7 @@ class Runner {
   /// Loads each suite in [suites] in order, pausing after load for platforms
   /// that support debugging.
   Future<bool> _loadThenPause(Stream<LoadSuite> suites) async {
-    if (_config.platforms.contains(TestPlatform.vm)) {
+    if (_config.suiteDefaults.platforms.contains(TestPlatform.vm)) {
       warn("Debugging is currently unsupported on the Dart VM.",
           color: _config.color);
     }

lib/src/runner/browser/browser_manager.dart

@@ -10,10 +10,10 @@ import 'package:pool/pool.dart';
 import 'package:stream_channel/stream_channel.dart';
 import 'package:web_socket_channel/web_socket_channel.dart';
 
-import '../../backend/metadata.dart';
 import '../../backend/test_platform.dart';
 import '../../util/stack_trace_mapper.dart';
 import '../application_exception.dart';
+import '../configuration/suite.dart';
 import '../environment.dart';
 import '../plugin/platform_helpers.dart';
 import '../runner_suite.dart';
@@ -193,14 +193,14 @@ class BrowserManager {
   ///
   /// [url] should be an HTML page with a reference to the JS-compiled test
   /// suite. [path] is the path of the original test suite file, which is used
-  /// for reporting. [metadata] is the parsed metadata for the test suite.
+  /// for reporting. [suiteConfig] is the configuration for the test suite.
   ///
   /// If [mapper] is passed, it's used to map stack traces for errors coming
   /// from this test suite.
-  Future<RunnerSuite> load(String path, Uri url, Metadata metadata,
+  Future<RunnerSuite> load(String path, Uri url, SuiteConfiguration suiteConfig,
       {StackTraceMapper mapper}) async {
     url = url.replace(fragment: Uri.encodeFull(JSON.encode({
-      "metadata": metadata.serialize(),
+      "metadata": suiteConfig.metadata.serialize(),
       "browser": _platform.identifier
     })));
 
@@ -235,7 +235,7 @@ class BrowserManager {
 
       try {
         controller = await deserializeSuite(
-            path, _platform, metadata, await _environment, suiteChannel,
+            path, _platform, suiteConfig, await _environment, suiteChannel,
             mapTrace: mapper?.mapStackTrace);
         _controllers.add(controller);
         return controller.suite;

lib/src/runner/browser/compiler_pool.dart

@@ -13,6 +13,7 @@ import 'package:pool/pool.dart';
 
 import '../../util/io.dart';
 import '../configuration.dart';
+import '../configuration/suite.dart';
 import '../load_exception.dart';
 
 /// A regular expression matching the first status line printed by dart2js.
@@ -47,7 +48,8 @@ class CompilerPool {
   ///
   /// The returned [Future] will complete once the `dart2js` process completes
   /// *and* all its output has been printed to the command line.
-  Future compile(String dartPath, String jsPath) {
+  Future compile(String dartPath, String jsPath,
+      SuiteConfiguration suiteConfig) {
     return _pool.withResource(() {
       if (_closed) return null;
 
@@ -75,7 +77,7 @@ class CompilerPool {
           wrapperPath,
           "--out=$jsPath",
           await PackageResolver.current.processArgument
-        ]..addAll(_config.dart2jsArgs);
+        ]..addAll(suiteConfig.dart2jsArgs);
 
         if (_config.color) args.add("--enable-diagnostic-colors");