From 0329aeb5062165ec26340e0bc3688c3964971bd5 Mon Sep 17 00:00:00 2001
From: Natalie Weizenbaum <nweiz@google.com>
Date: Mon, 29 Feb 2016 18:22:51 -0800
Subject: [PATCH] Add on_os and on_platform fields.

These allow configuration based on the user's OS and the platform the
test is running on.

R=kevmoo@google.com

Review URL: https://codereview.chromium.org//1730173004 .
---
 README.md                                     |   5 +
 doc/package_config.md                         |  81 +++++-
 lib/src/backend/metadata.dart                 |   1 +
 lib/src/runner/configuration.dart             |  20 +-
 lib/src/runner/configuration/load.dart        |  28 +-
 lib/src/utils.dart                            |   6 +-
 .../configuration/configuration_test.dart     |  23 ++
 test/runner/configuration/platform_test.dart  | 266 ++++++++++++++++++
 test/runner/configuration/tags_test.dart      |   2 +-
 test/runner/test_on_test.dart                 |   2 -
 10 files changed, 417 insertions(+), 17 deletions(-)
 create mode 100644 test/runner/configuration/platform_test.dart

diff --git a/README.md b/README.md
index b400ae37..b9a39f0d 100644
--- a/README.md
+++ b/README.md
@@ -500,6 +500,11 @@ If multiple platforms match, the configuration is applied in order from first to
 last, just as they would in nested groups. This means that for configuration
 like duration-based timeouts, the last matching value wins.
 
+You can also set up global platform-specific configuration using the
+[package configuration file][configuring platforms].
+
+[configuring platforms]: https://github.com/dart-lang/test/blob/master/doc/package_config.md#configuring-platforms
+
 ### Tagging Tests
 
 Tags are short strings that you can associate with tests, groups, and suites.
diff --git a/doc/package_config.md b/doc/package_config.md
index bead8c79..adf4057d 100644
--- a/doc/package_config.md
+++ b/doc/package_config.md
@@ -38,7 +38,11 @@ tags:
   * [`pub_serve`](#pub_serve)
   * [`reporter`](#reporter)
 * [Configuring Tags](#configuring-tags)
+  * [`tags`](#tags)
   * [`add_tags`](#add_tags)
+* [Configuring Platforms](#configuring-platforms)
+  * [`on_os`](#on_os)
+  * [`on_platform`](#on_platform)
 
 ## Test Configuration
 
@@ -47,8 +51,8 @@ configuration controls how individual tests run, while
 [runner configuration](#runner-configuration) controls the test runner as a
 whole. Both types of fields may be used at the top level of a configuration
 file. However, because different tests can have different test configuration,
-only test configuration fields may be used to
-[configure tags](#configuring-tags).
+only test configuration fields may be used to [configure tags](#tags) or
+[platforms](#on_platform).
 
 ### `timeout`
 
@@ -112,11 +116,12 @@ tags:
 ### `test_on`
 
 This field declares which platforms a test supports. It takes a
-[platform selector][] and only allows tests to run on platforms that match the
-selector. It's often used with [specific tags](#configuring-tags) to ensure that
-certain features will only be tested on supported platforms.
+[platform selector][platform selectors] and only allows tests to run on
+platforms that match the selector. It's often used with
+[specific tags](#configuring-tags) to ensure that certain features will only be
+tested on supported platforms.
 
-[platform selector]: https://github.com/dart-lang/test/blob/master/README.md#platform-selectors
+[platform selectors]: https://github.com/dart-lang/test/blob/master/README.md#platform-selectors
 
 ```yaml
 tags:
@@ -223,6 +228,8 @@ reporter: expanded
 
 ## Configuring Tags
 
+### `tags`
+
 The `tag` field can be used to apply [test configuration](#test-configuration)
 to all tests [with a given tag][tagging tests] or set of tags. It takes a map
 from tag selectors to configuration maps. These configuration maps are just like
@@ -274,6 +281,8 @@ configurations, the test runner *does not guarantee* what order they'll be
 resolved in. In practice, conflicting configuration is pretty unlikely and it's
 easy to just explicitly specify what you want on the test itself.
 
+This field counts as [test configuration](#test-configuration).
+
 ### `add_tags`
 
 This field adds additional tags. It's technically
@@ -295,3 +304,63 @@ tags:
   safari: {add_tags: [browser]}
   ie: {add_tags: [browser]}
 ```
+
+## Configuring Platforms
+
+There are two different kinds of platform configuration.
+[Operating system configuration](#on_os) cares about the operating system on
+which test runner is running. It sets global configuration for the runner on
+particular OSes. [Test platform configuration](#on_platform), on the other hand,
+cares about the platform the *test* is running on (like the
+[`@OnPlatform` annotation][@OnPlatform]). It sets configuration for particular
+tests that are running on matching platforms.
+
+[@OnPlatform]: https://github.com/dart-lang/test/blob/master/README.md#platform-specific-configuration
+
+### `on_os`
+
+This field applies configuration when specific operating systems are being used.
+It takes a map from operating system identifiers (the same ones that are used in
+[platform selectors][]) to configuration maps that are applied on those
+operating systems. These configuration maps are just like the top level of the
+configuration file, and allow any fields that may be used in the context where
+`on_os` was used.
+
+```yaml
+on_os:
+  windows:
+    # Both of these are the defaults anyway, but let's be explicit about it.
+    color: false
+    runner: expanded
+
+    # My Windows machine is real slow.
+    timeout: 2x
+
+  # My Linux machine has SO MUCH RAM.
+  linux:
+    concurrency: 500
+```
+
+This field counts as [test configuration](#test-configuration). If it's used in
+a context that only allows test configuration, it may only contain test
+configuration.
+
+### `on_platform`
+
+This field applies configuration to tests that are run on specific platforms. It
+takes a map from [platform selectors][] to configuration maps that are applied
+to tests run on those platforms. These configuration maps are just like the top
+level of the configuration file, except that they may not contain
+[runner configuration](#runner-configuration).
+
+```yaml
+# Our code is kind of slow on Blink and WebKit.
+on_platform:
+  chrome || safari: {timeout: 2x}
+```
+
+**Note**: operating system names that appear in `on_platform` refer to tests
+that are run on the Dart VM under that operating system. To configure all tests
+when running on a particular operating system, use [`on_os`](#on_os) instead.
+
+This field counts as [test configuration](#test-configuration).
diff --git a/lib/src/backend/metadata.dart b/lib/src/backend/metadata.dart
index 2b38c08e..6ff5412d 100644
--- a/lib/src/backend/metadata.dart
+++ b/lib/src/backend/metadata.dart
@@ -140,6 +140,7 @@ class Metadata {
     // If there's no tag-specific metadata, or if none of it applies, just
     // return the metadata as-is.
     if (forTag == null || tags == null) return _unresolved();
+    forTag = new Map.from(forTag);
 
     // Otherwise, resolve the tag-specific components. Doing this eagerly means
     // we only have to resolve suite- or group-level tags once, rather than
diff --git a/lib/src/runner/configuration.dart b/lib/src/runner/configuration.dart
index 8b673e44..aaeb94fb 100644
--- a/lib/src/runner/configuration.dart
+++ b/lib/src/runner/configuration.dart
@@ -139,7 +139,8 @@ class Configuration {
       skipReason: skipReason,
       testOn: testOn,
       tags: addTags,
-      forTag: mapMap(tags, value: (_, config) => config.metadata));
+      forTag: mapMap(tags, value: (_, config) => config.metadata),
+      onPlatform: mapMap(onPlatform, value: (_, config) => config.metadata));
 
   /// The set of tags that have been declaredin any way in this configuration.
   Set<String> get knownTags {
@@ -158,6 +159,13 @@ class Configuration {
   }
   Set<String> _knownTags;
 
+  /// Configuration for particular platforms.
+  ///
+  /// The keys are platform selectors, and the values are configurations for
+  /// those platforms. These configuration should only contain test-level
+  /// configuration fields, but that isn't enforced.
+  final Map<PlatformSelector, Configuration> onPlatform;
+
   /// Parses the configuration from [args].
   ///
   /// Throws a [FormatException] if [args] are invalid.
@@ -191,7 +199,8 @@ class Configuration {
           BooleanSelector includeTags,
           BooleanSelector excludeTags,
           Iterable addTags,
-          Map<BooleanSelector, Configuration> tags})
+          Map<BooleanSelector, Configuration> tags,
+          Map<PlatformSelector, Configuration> onPlatform})
       : _help = help,
         _version = version,
         _verboseTrace = verboseTrace,
@@ -215,7 +224,10 @@ class Configuration {
         includeTags = includeTags ?? BooleanSelector.all,
         excludeTags = excludeTags ?? BooleanSelector.none,
         addTags = addTags?.toSet() ?? new Set(),
-        tags = tags == null ? const {} : new Map.unmodifiable(tags) {
+        tags = tags == null ? const {} : new Map.unmodifiable(tags),
+        onPlatform = onPlatform == null
+            ? const {}
+            : new Map.unmodifiable(onPlatform) {
     if (_filename != null && _filename.context.style != p.style) {
       throw new ArgumentError(
           "filename's context must match the current operating system, was "
@@ -263,6 +275,8 @@ class Configuration {
         excludeTags: excludeTags.union(other.excludeTags),
         addTags: other.addTags.union(addTags),
         tags: mergeMaps(tags, other.tags,
+            value: (config1, config2) => config1.merge(config2)),
+        onPlatform: mergeMaps(onPlatform, other.onPlatform,
             value: (config1, config2) => config1.merge(config2)));
   }
 }
diff --git a/lib/src/runner/configuration/load.dart b/lib/src/runner/configuration/load.dart
index f66f0fc4..93cc7296 100644
--- a/lib/src/runner/configuration/load.dart
+++ b/lib/src/runner/configuration/load.dart
@@ -10,10 +10,12 @@ import 'package:path/path.dart' as p;
 import 'package:source_span/source_span.dart';
 import 'package:yaml/yaml.dart';
 
+import '../../backend/operating_system.dart';
 import '../../backend/platform_selector.dart';
 import '../../backend/test_platform.dart';
 import '../../frontend/timeout.dart';
 import '../../utils.dart';
+import '../../util/io.dart';
 import '../configuration.dart';
 import 'values.dart';
 
@@ -88,7 +90,25 @@ class _ConfigurationLoader {
         value: (valueNode) =>
             _nestedConfig(valueNode, "tag value", runnerConfig: false));
 
-    return new Configuration(
+    var onPlatform = _getMap("on_platform",
+        key: (keyNode) => _parseNode(keyNode, "on_platform key",
+            (value) => new PlatformSelector.parse(value)),
+        value: (valueNode) =>
+            _nestedConfig(valueNode, "on_platform value", runnerConfig: false));
+
+    var onOS = _getMap("on_os", key: (keyNode) {
+      _validate(keyNode, "on_os key must be a string.",
+          (value) => value is String);
+
+      var os = OperatingSystem.find(keyNode.value);
+      if (os != null) return os;
+
+      throw new SourceSpanFormatException(
+          'Invalid on_os key: No such operating system.',
+          keyNode.span, _source);
+    }, value: (valueNode) => _nestedConfig(valueNode, "on_os value"));
+
+    var config = new Configuration(
         verboseTrace: verboseTrace,
         jsTrace: jsTrace,
         skip: skip,
@@ -96,7 +116,11 @@ class _ConfigurationLoader {
         testOn: testOn,
         timeout: timeout,
         addTags: addTags,
-        tags: tags);
+        tags: tags,
+        onPlatform: onPlatform);
+
+    var osConfig = onOS[currentOS];
+    return osConfig == null ? config : config.merge(osConfig);
   }
 
   /// Loads runner configuration (but not test configuration).
diff --git a/lib/src/utils.dart b/lib/src/utils.dart
index 6d54faf0..73e42cb3 100644
--- a/lib/src/utils.dart
+++ b/lib/src/utils.dart
@@ -228,11 +228,11 @@ Map mapMap(Map map, {key(key, value), value(key, value)}) {
 /// not passed, [map2]'s value wins.
 Map mergeMaps(Map map1, Map map2, {value(value1, value2)}) {
   var result = new Map.from(map1);
-  map2.forEach((key, value) {
+  map2.forEach((key, mapValue) {
     if (value == null || !result.containsKey(key)) {
-      result[key] = value;
+      result[key] = mapValue;
     } else {
-      result[key] = value(result[key], value);
+      result[key] = value(result[key], mapValue);
     }
   });
   return result;
diff --git a/test/runner/configuration/configuration_test.dart b/test/runner/configuration/configuration_test.dart
index 45e29af0..fa56479b 100644
--- a/test/runner/configuration/configuration_test.dart
+++ b/test/runner/configuration/configuration_test.dart
@@ -263,5 +263,28 @@ void main() {
         expect(merged.timeout, equals(new Timeout(new Duration(seconds: 2))));
       });
     });
+
+    group("for onPlatform", () {
+      test("merges each nested configuration", () {
+        var merged = new Configuration(
+          onPlatform: {
+            new PlatformSelector.parse("vm"): new Configuration(verboseTrace: true),
+            new PlatformSelector.parse("chrome"): new Configuration(jsTrace: true)
+          }
+        ).merge(new Configuration(
+          onPlatform: {
+            new PlatformSelector.parse("chrome"): new Configuration(jsTrace: false),
+            new PlatformSelector.parse("firefox"): new Configuration(skip: true)
+          }
+        ));
+
+        expect(merged.onPlatform[new PlatformSelector.parse("vm")].verboseTrace,
+            isTrue);
+        expect(merged.onPlatform[new PlatformSelector.parse("chrome")].jsTrace,
+            isFalse);
+        expect(merged.onPlatform[new PlatformSelector.parse("firefox")].skip,
+            isTrue);
+      });
+    });
   });
 }
diff --git a/test/runner/configuration/platform_test.dart b/test/runner/configuration/platform_test.dart
new file mode 100644
index 00000000..f9bef940
--- /dev/null
+++ b/test/runner/configuration/platform_test.dart
@@ -0,0 +1,266 @@
+// Copyright (c) 2016, the Dart project authors.  Please see the AUTHORS file
+// 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.
+
+@TestOn("vm")
+
+import 'dart:convert';
+
+import 'package:scheduled_test/descriptor.dart' as d;
+import 'package:scheduled_test/scheduled_test.dart';
+
+import 'package:test/src/util/exit_codes.dart' as exit_codes;
+import 'package:test/src/util/io.dart';
+
+import '../../io.dart';
+
+void main() {
+  useSandbox();
+
+  group("on_platform", () {
+    test("applies platform-specific configuration to matching tests", () {
+      d.file("dart_test.yaml", JSON.encode({
+        "on_platform": {
+          "content-shell": {"timeout": "0s"}
+        }
+      })).create();
+
+      d.file("test.dart", """
+        import 'dart:async';
+
+        import 'package:test/test.dart';
+
+        void main() {
+          test("test", () => new Future.delayed(Duration.ZERO));
+        }
+      """).create();
+
+      var test = runTest(["-p", "content-shell,vm", "test.dart"]);
+      test.stdout.expect(containsInOrder([
+        "-1: [Dartium Content Shell] test",
+        "+1 -1: Some tests failed."
+      ]));
+      test.shouldExit(1);
+    }, tags: ['content-shell']);
+
+    test("supports platform selectors", () {
+      d.file("dart_test.yaml", JSON.encode({
+        "on_platform": {
+          "content-shell || vm": {"timeout": "0s"}
+        }
+      })).create();
+
+      d.file("test.dart", """
+        import 'dart:async';
+
+        import 'package:test/test.dart';
+
+        void main() {
+          test("test", () => new Future.delayed(Duration.ZERO));
+        }
+      """).create();
+
+      var test = runTest(["-p", "content-shell,vm", "test.dart"]);
+      test.stdout.expect(containsInOrder([
+        "-1: [VM] test",
+        "-2: [Dartium Content Shell] test",
+        "-2: Some tests failed."
+      ]));
+      test.shouldExit(1);
+    }, tags: ['content-shell']);
+
+    group("errors", () {
+      test("rejects an invalid selector type", () {
+        d.file("dart_test.yaml", '{"on_platform": {12: null}}').create();
+
+        var test = runTest([]);
+        test.stderr.expect(containsInOrder([
+          "on_platform key must be a string",
+          "^^"
+        ]));
+        test.shouldExit(exit_codes.data);
+      });
+
+      test("rejects an invalid selector", () {
+        d.file("dart_test.yaml", JSON.encode({
+          "on_platform": {"foo bar": null}
+        })).create();
+
+        var test = runTest([]);
+        test.stderr.expect(containsInOrder([
+          "Invalid on_platform key: Expected end of input.",
+          "^^^^^^^^^"
+        ]));
+        test.shouldExit(exit_codes.data);
+      });
+
+      test("rejects a selector with an undefined variable", () {
+        d.file("dart_test.yaml", JSON.encode({
+          "on_platform": {"foo": null}
+        })).create();
+
+        var test = runTest([]);
+        test.stderr.expect(containsInOrder([
+          "Invalid on_platform key: Undefined variable.",
+          "^^^^^"
+        ]));
+        test.shouldExit(exit_codes.data);
+      });
+
+      test("rejects an invalid map", () {
+        d.file("dart_test.yaml", JSON.encode({
+          "on_platform": {"linux": 12}
+        })).create();
+
+        var test = runTest([]);
+        test.stderr.expect(containsInOrder([
+          "on_platform value must be a map.",
+          "^^"
+        ]));
+        test.shouldExit(exit_codes.data);
+      });
+
+      test("rejects an invalid configuration", () {
+        d.file("dart_test.yaml", JSON.encode({
+          "on_platform": {"linux": {"timeout": "12p"}}
+        })).create();
+
+        var test = runTest([]);
+        test.stderr.expect(containsInOrder([
+          "Invalid timeout: expected unit.",
+          "^^^^^"
+        ]));
+        test.shouldExit(exit_codes.data);
+      });
+
+      test("rejects runner configuration", () {
+        d.file("dart_test.yaml", JSON.encode({
+          "on_platform": {"linux": {"filename": "*_blorp"}}
+        })).create();
+
+        var test = runTest([]);
+        test.stderr.expect(containsInOrder([
+          "filename isn't supported here.",
+          "^^^^^^^^^"
+        ]));
+        test.shouldExit(exit_codes.data);
+      });
+    });
+  });
+
+  group("on_os", () {
+    test("applies OS-specific configuration on a matching OS", () {
+      d.file("dart_test.yaml", JSON.encode({
+        "on_os": {
+          currentOS.identifier: {"filename": "test_*.dart"}
+        }
+      })).create();
+
+      d.file("foo_test.dart", """
+        import 'package:test/test.dart';
+
+        void main() {
+          test("foo_test", () {});
+        }
+      """).create();
+
+      d.file("test_foo.dart", """
+        import 'package:test/test.dart';
+
+        void main() {
+          test("test_foo", () {});
+        }
+      """).create();
+
+      var test = runTest(["."]);
+      test.stdout.expect(containsInOrder([
+        "+0: ./test_foo.dart: test_foo",
+        "+1: All tests passed!"
+      ]));
+      test.shouldExit(0);
+    });
+
+    test("doesn't OS-specific configuration on a non-matching OS", () {
+      d.file("dart_test.yaml", JSON.encode({
+        "on_os": {
+          otherOS: {"filename": "test_*.dart"}
+        }
+      })).create();
+
+      d.file("foo_test.dart", """
+        import 'package:test/test.dart';
+
+        void main() {
+          test("foo_test", () {});
+        }
+      """).create();
+
+      d.file("test_foo.dart", """
+        import 'package:test/test.dart';
+
+        void main() {
+          test("test_foo", () {});
+        }
+      """).create();
+
+      var test = runTest(["."]);
+      test.stdout.expect(containsInOrder([
+        "+0: ./foo_test.dart: foo_test",
+        "+1: All tests passed!"
+      ]));
+      test.shouldExit(0);
+    });
+
+    group("errors", () {
+      test("rejects an invalid OS type", () {
+        d.file("dart_test.yaml", '{"on_os": {12: null}}').create();
+
+        var test = runTest([]);
+        test.stderr.expect(containsInOrder([
+          "on_os key must be a string",
+          "^^"
+        ]));
+        test.shouldExit(exit_codes.data);
+      });
+
+      test("rejects an unknown OS name", () {
+        d.file("dart_test.yaml", JSON.encode({
+          "on_os": {"foo": null}
+        })).create();
+
+        var test = runTest([]);
+        test.stderr.expect(containsInOrder([
+          "Invalid on_os key: No such operating system.",
+          "^^^^^"
+        ]));
+        test.shouldExit(exit_codes.data);
+      });
+
+      test("rejects an invalid map", () {
+        d.file("dart_test.yaml", JSON.encode({
+          "on_os": {"linux": 12}
+        })).create();
+
+        var test = runTest([]);
+        test.stderr.expect(containsInOrder([
+          "on_os value must be a map.",
+          "^^"
+        ]));
+        test.shouldExit(exit_codes.data);
+      });
+
+      test("rejects an invalid configuration", () {
+        d.file("dart_test.yaml", JSON.encode({
+          "on_os": {"linux": {"timeout": "12p"}}
+        })).create();
+
+        var test = runTest([]);
+        test.stderr.expect(containsInOrder([
+          "Invalid timeout: expected unit.",
+          "^^^^^"
+        ]));
+        test.shouldExit(exit_codes.data);
+      });
+    });
+  });
+}
diff --git a/test/runner/configuration/tags_test.dart b/test/runner/configuration/tags_test.dart
index 0376a959..cddbf761 100644
--- a/test/runner/configuration/tags_test.dart
+++ b/test/runner/configuration/tags_test.dart
@@ -171,7 +171,7 @@ void main() {
         test.shouldExit(exit_codes.data);
       });
 
-      test("rejects an inavlid tag configuration", () {
+      test("rejects an invalid tag configuration", () {
         d.file("dart_test.yaml", JSON.encode({
           "tags": {"foo": {"timeout": "12p"}}
         })).create();
diff --git a/test/runner/test_on_test.dart b/test/runner/test_on_test.dart
index c739bfe6..99b7986c 100644
--- a/test/runner/test_on_test.dart
+++ b/test/runner/test_on_test.dart
@@ -4,8 +4,6 @@
 
 @TestOn("vm")
 
-import 'dart:io';
-
 import 'package:scheduled_test/descriptor.dart' as d;
 import 'package:scheduled_test/scheduled_stream.dart';
 import 'package:scheduled_test/scheduled_test.dart';
-- 
GitLab