Respect top-level @TestOn declarations.

Closes #6

R=kevmoo@google.com

Review URL: https://codereview.chromium.org//1027193004

README.md

@@ -95,6 +95,77 @@ the results will be reported on the command line just like for VM tests. In
 fact, you can even run tests on both platforms with a single command: `pub run
 unittest:unittest -p chrome -p vm path/to/test.dart`.
 
+### Restricting Tests to Certain Platforms
+
+Some test files only make sense to run on particular platforms. They may use
+`dart:html` or `dart:io`, they might test Windows' particular filesystem
+behavior, or they might use a feature that's only available in Chrome. The
+[`@TestOn`][TestOn] annotation makes it easy to declare exactly which platforms
+a test file should run on. Just put it at the top of your file, before any
+`library` or `import` declarations:
+
+```dart
+@TestOn("vm")
+
+import "dart:io";
+
+import "package:unittest/unittest.dart";
+
+void main() {
+  // ...
+}
+```
+
+[TestOn]: http://www.dartdocs.org/documentation/unittest/latest/index.html#unittest/unittest.TestOn
+
+The string you pass to `@TestOn` is what's called a "platform selector", and it
+specifies exactly which platforms a test can run on. It can be as simple as the
+name of a platform, or a more complex Dart-like boolean expression involving
+these platform names.
+
+### Platform Selector Syntax
+
+Platform selectors can contain identifiers, parentheses, and operators. When
+loading a test, each identifier is set to `true` or `false` based on the current
+platform, and the test is only loaded if the platform selector returns `true`.
+The operators `||`, `&&`, `!`, and `? :` all work just like they do in Dart. The
+valid identifiers are:
+
+* `vm`: Whether the test is running on the command-line Dart VM.
+
+* `chrome`: Whether the test is running on Google Chrome.
+
+* `dart-vm`: Whether the test is running on the Dart VM in any context. For now
+  this is identical to `vm`, but it will also be true for Dartium in the future.
+  It's identical to `!js`.
+
+* `browser`: Whether the test is running in any browser.
+
+* `js`: Whether the test has been compiled to JS. This is identical to
+  `!dart-vm`.
+
+* `blink`: Whether the test is running in a browser that uses the Blink
+  rendering engine.
+
+* `windows`: Whether the test is running on Windows. If `vm` is false, this will
+  be `false` as well.
+
+* `mac-os`: Whether the test is running on Mac OS. If `vm` is false, this will
+  be `false` as well.
+
+* `linux`: Whether the test is running on Linux. If `vm` is false, this will be
+  `false` as well.
+
+* `android`: Whether the test is running on Android. If `vm` is false, this will
+  be `false` as well, which means that this *won't* be true if the test is
+  running on an Android browser.
+
+* `posix`: Whether the test is running on a POSIX operating system. This is
+  equivalent to `!windows`.
+
+For example, if you wanted to run a test on every browser but Chrome, you would
+write `@TestOn("browser && !chrome")`.
+
 ## Asynchronous Tests
 
 Tests written with `async`/`await` will work automatically. The test runner

bin/unittest.dart

@@ -78,12 +78,12 @@ void main(List<String> args) {
     }).whenComplete(() => reporter.close());
   }).catchError((error, stackTrace) {
     if (error is LoadException) {
-      // TODO(nweiz): color this message?
-      stderr.writeln(getErrorMessage(error));
+      stderr.writeln(error.toString(color: color));
 
       // Only print stack traces for load errors that come from the user's 
       if (error.innerError is! IOException &&
           error.innerError is! IsolateSpawnException &&
+          error.innerError is! FormatException &&
           error.innerError is! String) {
         stderr.write(terseChain(stackTrace));
       }

lib/src/backend/metadata.dart

@@ -4,13 +4,26 @@
 
 library unittest.backend.metadata;
 
+import 'platform_selector.dart';
+
 /// Metadata for a test or test suite.
 ///
 /// This metadata comes from declarations on the test itself; it doesn't include
 /// configuration from the user.
 class Metadata {
-  /// The expressions indicating which platforms the suite supports.
-  final String testOn;
+  /// The selector indicating which platforms the suite supports.
+  final PlatformSelector testOn;
+
+  /// Creates new Metadata.
+  ///
+  /// [testOn] defaults to [PlatformSelector.all].
+  Metadata({PlatformSelector testOn})
+      : testOn = testOn == null ? PlatformSelector.all : testOn;
 
-  Metadata(this.testOn);
+  /// Parses metadata fields from strings.
+  ///
+  /// Throws a [FormatException] if any field is invalid.
+  Metadata.parse({String testOn})
+      : this(
+          testOn: testOn == null ? null : new PlatformSelector.parse(testOn));
 }

lib/src/backend/platform_selector.dart

@@ -23,25 +23,57 @@ final _validVariables =
 /// and browsers.
 ///
 /// The syntax is mostly Dart's expression syntax restricted to boolean
-/// operations. See the README for full details.
-class PlatformSelector {
-  /// The parsed AST.
-  final Node _selector;
+/// operations. See [the README][] for full details.
+///
+/// [the README]: https://github.com/dart-lang/unittest/#platform-selector-syntax
+abstract class PlatformSelector {
+  /// A selector that declares that a test can be run on all platforms.
+  ///
+  /// This isn't representable in the platform selector syntax but it is the
+  /// default selector.
+  static const all = const _AllPlatforms();
 
   /// Parses [selector].
   ///
   /// This will throw a [SourceSpanFormatException] if the selector is
   /// malformed or if it uses an undefined variable.
-  PlatformSelector.parse(String selector)
-      : _selector = new Parser(selector).parse() {
-    _selector.accept(const _VariableValidator());
-  }
+  factory PlatformSelector.parse(String selector) =>
+      new _PlatformSelector.parse(selector);
 
   /// Returns whether the selector matches the given [platform] and [os].
   ///
   /// [os] defaults to [OperatingSystem.none].
+  bool evaluate(TestPlatform platform, {OperatingSystem os});
+}
+
+/// The concrete implementation of a [PlatformSelector] parsed from a string.
+///
+/// This is separate from [PlatformSelector] so that [_AllPlatforms] can
+/// implement [PlatformSelector] without having to implement private members.
+class _PlatformSelector implements PlatformSelector{
+  /// The parsed AST.
+  final Node _selector;
+
+  _PlatformSelector.parse(String selector)
+      : _selector = new Parser(selector).parse() {
+    _selector.accept(const _VariableValidator());
+  }
+
+  _PlatformSelector(this._selector);
+
   bool evaluate(TestPlatform platform, {OperatingSystem os}) =>
       _selector.accept(new Evaluator(platform, os: os));
+
+  String toString() => _selector.toString();
+}
+
+/// A selector that matches all platforms.
+class _AllPlatforms implements PlatformSelector {
+  const _AllPlatforms();
+
+  bool evaluate(TestPlatform platform, {OperatingSystem os}) => true;
+
+  String toString() => "*";
 }
 
 /// An AST visitor that ensures that all variables are valid.

lib/src/backend/suite.dart

@@ -6,6 +6,7 @@ library unittest.backend.suite;
 
 import 'dart:collection';
 
+import 'metadata.dart';
 import 'test.dart';
 
 /// A test suite.
@@ -20,11 +21,23 @@ class Suite {
   /// The path to the Dart test suite, or `null` if that path is unknown.
   final String path;
 
+  /// The metadata associated with this test suite.
+  final Metadata metadata;
+
   /// The tests in the test suite.
   final List<Test> tests;
 
-  Suite(Iterable<Test> tests, {String path, String platform})
-      : path = path,
-        platform = platform,
+  Suite(Iterable<Test> tests, {this.path, this.platform, Metadata metadata})
+      : metadata = metadata == null ? new Metadata() : metadata,
         tests = new UnmodifiableListView<Test>(tests.toList());
+
+  /// Returns a new suite with the given fields updated.
+  Suite change({String path, String platform, Metadata metadata,
+      Iterable<Test> tests}) {
+    if (path == null) path = this.path;
+    if (platform == null) platform = this.platform;
+    if (metadata == null) metadata = this.metadata;
+    if (tests == null) tests = this.tests;
+    return new Suite(tests, path: path, platform: platform, metadata: metadata);
+  }
 }

lib/src/frontend/test_on.dart

@@ -4,11 +4,11 @@
 
 library unittest.frontend.test_on;
 
-/// An annotation indicating which platforms a test or test suite supports.
+/// An annotation indicating which platforms a test suite supports.
 ///
-/// For the full syntax of [expression], see [the README][readme].
+/// For the full syntax of [expression], see [the README][].
 ///
-/// [readme]: https://github.com/dart-lang/unittest/#readme
+/// [the README]: https://github.com/dart-lang/unittest/#platform-selector-syntax
 class TestOn {
   /// The expression specifying the platform.
   final String expression;

lib/src/runner/load_exception.dart

@@ -7,6 +7,7 @@ library unittest.runner.load_exception;
 import 'dart:isolate';
 
 import 'package:path/path.dart' as p;
+import 'package:source_span/source_span.dart';
 
 import '../utils.dart';
 
@@ -17,8 +18,11 @@ class LoadException implements Exception {
 
   LoadException(this.path, this.innerError);
 
-  String toString() {
-    var buffer = new StringBuffer('Failed to load "$path":');
+  String toString({bool color: false}) {
+    var buffer = new StringBuffer();
+    if (color) buffer.write('\u001b[31m'); // red
+    buffer.write('Failed to load "$path":');
+    if (color) buffer.write('\u001b[0m'); // no color
 
     var innerString = getErrorMessage(innerError);
     if (innerError is IsolateSpawnException) {
@@ -33,6 +37,9 @@ class LoadException implements Exception {
           "Uncaught Error: Load Error: FileSystemException: ",
           "");
       innerString = innerString.split("Stack Trace:\n").first.trim();
+    } if (innerError is SourceSpanException) {
+      innerString = innerError.toString(color: color)
+          .replaceFirst(" of $path", "");
     }
 
     buffer.write(innerString.contains("\n") ? "\n" : " ");

lib/src/runner/loader.dart

@@ -8,8 +8,10 @@ import 'dart:async';
 import 'dart:io';
 import 'dart:isolate';
 
+import 'package:analyzer/analyzer.dart';
 import 'package:path/path.dart' as p;
 
+import '../backend/metadata.dart';
 import '../backend/suite.dart';
 import '../backend/test_platform.dart';
 import '../util/dart.dart';
@@ -18,6 +20,7 @@ import '../util/remote_exception.dart';
 import '../utils.dart';
 import 'browser/server.dart';
 import 'load_exception.dart';
+import 'parse_metadata.dart';
 import 'vm/isolate_test.dart';
 
 /// A class for finding test files and loading them into a runnable form.
@@ -82,11 +85,27 @@ class Loader {
   ///
   /// This will throw a [LoadException] if the file fails to load.
   Future<List<Suite>> loadFile(String path) {
+    var metadata;
+    try {
+      metadata = parseMetadata(path);
+    } on AnalyzerErrorGroup catch (_) {
+      // Ignore the analyzer's error, since its formatting is much worse than
+      // the VM's or dart2js's.
+      metadata = new Metadata();
+    } on FormatException catch (error) {
+      throw new LoadException(path, error);
+    }
+
     return Future.wait(_platforms.map((platform) {
-      if (platform == TestPlatform.chrome) return _loadBrowserFile(path);
-      assert(platform == TestPlatform.vm);
-      return _loadVmFile(path);
-    }));
+      return new Future.sync(() {
+        if (!metadata.testOn.evaluate(platform, os: currentOS)) return null;
+
+        if (platform == TestPlatform.chrome) return _loadBrowserFile(path);
+        assert(platform == TestPlatform.vm);
+        return _loadVmFile(path);
+      }).then((suite) =>
+          suite == null ? null : suite.change(metadata: metadata));
+    })).then((suites) => suites.where((suite) => suite != null).toList());
   }
 
   /// Load the test suite at [path] in a browser.

lib/src/runner/parse_metadata.dart

@@ -99,7 +99,7 @@ Metadata parseMetadata(String path) {
     testOn = args.first.stringValue;
   }
 
-  return new Metadata(testOn);
+  return new Metadata.parse(testOn: testOn);
 }
 
 /// Creates a [SourceSpan] for [node].

lib/src/util/io.dart

@@ -10,12 +10,22 @@ import 'dart:mirrors';
 
 import 'package:path/path.dart' as p;
 
+import '../backend/operating_system.dart';
 import '../runner/load_exception.dart';
 
 /// The root directory of the Dart SDK.
 final String sdkDir =
     p.dirname(p.dirname(Platform.executable));
 
+/// Returns the current operating system.
+final OperatingSystem currentOS = (() {
+  var name = Platform.operatingSystem;
+  var os = OperatingSystem.findByIoName(name);
+  if (os != null) return os;
+
+  throw new UnsupportedError('Unsupported operating system "$name".');
+})();
+
 /// The path to the `lib` directory of the `unittest` package.
 String libDir({String packageRoot}) {
   var pathToIo = libraryPath(#unittest.util.io, packageRoot: packageRoot);

lib/unittest.dart

@@ -24,6 +24,7 @@ export 'src/frontend/expect.dart';
 export 'src/frontend/expect_async.dart';
 export 'src/frontend/future_matchers.dart';
 export 'src/frontend/prints_matcher.dart';
+export 'src/frontend/test_on.dart';
 export 'src/frontend/throws_matcher.dart';
 export 'src/frontend/throws_matchers.dart';
 

test/backend/platform_selector/evaluate_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.
 
+@TestOn("vm")
+
 import 'dart:io';
 
 import 'package:unittest/unittest.dart';

test/runner/browser/chrome_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.
 
+@TestOn("vm")
+
 import 'dart:async';
 import 'dart:io';
 

test/runner/browser/compact_reporter_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.
 
+@TestOn("vm")
+
 import 'dart:io';
 
 import 'package:path/path.dart' as p;

test/runner/browser/compiler_pool_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.
 
+@TestOn("vm")
+
 import 'dart:io';
 
 import 'package:path/path.dart' as p;

test/runner/browser/loader_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.
 
+@TestOn("vm")
+
 import 'dart:io';
 
 import 'package:path/path.dart' as p;

test/runner/browser/runner_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.
 
+@TestOn("vm")
+
 import 'dart:io';
 
 import 'package:path/path.dart' as p;

test/runner/compact_reporter_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.
 
+@TestOn("vm")
+
 import 'dart:io';
 
 import 'package:path/path.dart' as p;

test/runner/isolate_listener_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.
 
+@TestOn("vm")
+
 import 'dart:async';
 import 'dart:isolate';
 

test/runner/loader_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.
 
+@TestOn("vm")
+
 import 'dart:io';
 
 import 'package:path/path.dart' as p;
@@ -84,14 +86,11 @@ void main() {
 
     test("throws a nice error if the package root doesn't exist", () {
       var loader = new Loader([TestPlatform.vm]);
-      expect(() {
-        try {
-          loader.loadFile(p.join(_sandbox, 'a_test.dart'));
-        } finally {
-          loader.close();
-        }
-      }, throwsA(isLoadException(
-          "Directory ${p.join(_sandbox, 'packages')} does not exist.")));
+      expect(
+          loader.loadFile(p.join(_sandbox, 'a_test.dart'))
+              .whenComplete(loader.close),
+          throwsA(isLoadException(
+              "Directory ${p.join(_sandbox, 'packages')} does not exist.")));
     });
   });