Support @Skip as a suite-level annotation.

Closes #14

R=kevmoo@google.com

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

CHANGELOG.md

@@ -5,6 +5,12 @@
 
 [custom html]: https://github.com/dart-lang/test/blob/master/README.md#running-tests-with-custom-html
 
+* Tests, groups, and suites may be declared as skipped. Tests and groups are
+  skipped using the `skip` named argument; suites are skipped using the `@Skip`
+  annotation. See [the README][skip] for more information.
+
+[skip]: https://github.com/dart-lang/test/blob/master/README.md#skipping-tests
+
 * Fix running VM tests against `pub serve`.
 
 * More gracefully handle browser errors.

README.md

@@ -315,6 +315,49 @@ the following HTML file:
 
 ## Configuring Tests
 
+### Skipping Tests
+
+If a test, group, or entire suite isn't working yet and you just want it to stop
+complaining, you can mark it as "skipped". The test or tests won't be run, and,
+if you supply a reason why, that reason will be printed. In general, skipping
+tests indicates that they should run but is temporarily not working. If they're
+is fundamentally incompatible with a platform, [`@TestOn`/`testOn`][TestOn]
+should be used instead.
+
+[TestOn]: #restricting-tests-to-certain-platforms
+
+To skip a test suite, put a `@Skip` annotation at the top of the file:
+
+```dart
+@Skip("currently failing (see issue 1234)")
+
+import "package:test/test.dart";
+
+void main() {
+  // ...
+}
+```
+
+The string you pass should describe why the test is skipped. You don't have to
+include it, but it's a good idea to document why the test isn't running.
+
+Groups and individual tests can be skipped by passing the `skip` parameter. This
+can be either `true` or a String describing why the test is skipped. For example:
+
+```dart
+import "package:test/test.dart";
+
+void main() {
+  group("complicated algorithm tests", () {
+    // ...
+  }, skip: "the algorithm isn't quite right");
+  
+  test("error-checking test", () {
+    // ...
+  }, skip: "TODO: add error-checking.");
+}
+```
+
 ### Timeouts
 
 By default, tests will time out after 30 seconds of inactivity. However, this

lib/src/frontend/skip.dart

+// Copyright (c) 2015, 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.
+
+library test.frontend.skip;
+
+/// An annotation for marking a test suite as skipped.
+class Skip {
+  /// The reason the test suite is skipped, or `null` if no reason is given.
+  final String reason;
+
+  /// Marks a suite as skipped.
+  ///
+  /// If [reason] is passed, it's included in the test output as the reason the
+  /// test is skipped.
+  const Skip([this.reason]);
+}

lib/src/runner/loader.dart

@@ -127,14 +127,15 @@ class Loader {
     var controller = new StreamController();
     Future.forEach(_platforms, (platform) {
       if (!metadata.testOn.evaluate(platform, os: currentOS)) {
-        return new Future.value();
+        return;
       }
 
       // Don't load a skipped suite.
       if (metadata.skip) {
-        return new Future.value(new Suite([
+        controller.add(new Suite([
           new LocalTest(path, metadata, () {})
         ], path: path, platform: platform.name, metadata: metadata));
+        return;
       }
 
       return new Future.sync(() {

lib/src/runner/parse_metadata.dart

@@ -32,6 +32,7 @@ const _durationArgs = const [
 Metadata parseMetadata(String path) {
   var timeout;
   var testOn;
+  var skip;
 
   var contents = new File(path).readAsStringSync();
   var directives = parseDirectives(contents, name: path).directives;
@@ -82,13 +83,21 @@ Metadata parseMetadata(String path) {
             _spanFor(annotation, path));
       }
       timeout = _parseTimeout(annotation, constructorName, path);
+    } else if (name == 'Skip') {
+      if (skip != null) {
+        throw new SourceSpanFormatException(
+            "Only a single Skip annotation may be used for a given test file.",
+            _spanFor(annotation, path));
+      }
+      skip = _parseSkip(annotation, constructorName, path);
     }
   }
 
   try {
     return new Metadata.parse(
         testOn: testOn == null ? null : testOn.stringValue,
-        timeout: timeout);
+        timeout: timeout,
+        skip: skip);
   } on SourceSpanFormatException catch (error) {
     var file = new SourceFile(new File(path).readAsStringSync(),
         url: p.toUri(path));
@@ -188,6 +197,47 @@ Timeout _parseTimeout(Annotation annotation, String constructorName,
   }
 }
 
+/// Parses a `@Skip` annotation.
+///
+/// [annotation] is the annotation. [constructorName] is the name of the named
+/// constructor for the annotation, if any. [path] is the path to the file from
+/// which the annotation was parsed.
+///
+/// Returns either `true` or a reason string.
+_parseSkip(Annotation annotation, String constructorName, String path) {
+  if (constructorName != null) {
+    throw new SourceSpanFormatException(
+        'Skip doesn\'t have a constructor named "$constructorName".',
+        _spanFor(annotation, path));
+  }
+
+  if (annotation.arguments == null) {
+    throw new SourceSpanFormatException(
+        'Skip must have parentheses.', _spanFor(annotation, path));
+  }
+
+  var args = annotation.arguments.arguments;
+  if (args.length > 1) {
+    throw new SourceSpanFormatException(
+        'Skip takes zero arguments or one argument.',
+        _spanFor(annotation.arguments, path));
+  }
+
+  if (args.isEmpty) return true;
+
+  if (args.first is NamedExpression) {
+    throw new SourceSpanFormatException(
+        "Skip doesn't take named parameters.", _spanFor(args.first, path));
+  }
+
+  if (args.first is! StringLiteral) {
+    throw new SourceSpanFormatException(
+        "Skip takes a String.", _spanFor(args.first, path));
+  }
+
+  return args.first.stringValue;
+}
+
 /// Parses a `const Duration` expression.
 Duration _parseDuration(Expression expression, String path) {
   if (expression is! InstanceCreationExpression) {

lib/test.dart

@@ -22,6 +22,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/skip.dart';
 export 'src/frontend/test_on.dart';
 export 'src/frontend/throws_matcher.dart';
 export 'src/frontend/throws_matchers.dart';

test/runner/parse_metadata_test.dart

@@ -228,4 +228,60 @@ library foo;
       });
     });
   });
+
+  group("@Skip:", () {
+    test("parses a valid annotation", () {
+      new File(_path).writeAsStringSync("@Skip()\nlibrary foo;");
+      var metadata = parseMetadata(_path);
+      expect(metadata.skip, isTrue);
+      expect(metadata.skipReason, isNull);
+    });
+
+    test("parses a valid annotation with a reason", () {
+      new File(_path).writeAsStringSync("@Skip('reason')\nlibrary foo;");
+      var metadata = parseMetadata(_path);
+      expect(metadata.skip, isTrue);
+      expect(metadata.skipReason, equals('reason'));
+    });
+
+    test("ignores a constructor named Skip", () {
+      new File(_path).writeAsStringSync("@foo.Skip('foo')\nlibrary foo;");
+      var metadata = parseMetadata(_path);
+      expect(metadata.skip, isFalse);
+    });
+
+    group("throws an error for", () {
+      test("a named constructor", () {
+        new File(_path).writeAsStringSync("@Skip.name('foo')\nlibrary foo;");
+        expect(() => parseMetadata(_path), throwsFormatException);
+      });
+
+      test("no argument list", () {
+        new File(_path).writeAsStringSync("@Skip\nlibrary foo;");
+        expect(() => parseMetadata(_path), throwsFormatException);
+      });
+
+      test("a named argument", () {
+        new File(_path).writeAsStringSync(
+            "@Skip(reason: 'foo')\nlibrary foo;");
+        expect(() => parseMetadata(_path), throwsFormatException);
+      });
+
+      test("multiple arguments", () {
+        new File(_path).writeAsStringSync("@Skip('foo', 'bar')\nlibrary foo;");
+        expect(() => parseMetadata(_path), throwsFormatException);
+      });
+
+      test("a non-string argument", () {
+        new File(_path).writeAsStringSync("@Skip(123)\nlibrary foo;");
+        expect(() => parseMetadata(_path), throwsFormatException);
+      });
+
+      test("multiple @Skips", () {
+        new File(_path).writeAsStringSync(
+            "@Skip('foo')\n@Skip('bar')\nlibrary foo;");
+        expect(() => parseMetadata(_path), throwsFormatException);
+      });
+    });
+  });
 }
\ No newline at end of file

test/runner/runner_test.dart

@@ -351,6 +351,23 @@ void main() {
     expect(result.stdout, contains("-1: Some tests failed."));
   });
 
+  test("respects top-level @Skip declarations", () {
+    new File(p.join(_sandbox, "test.dart")).writeAsStringSync('''
+@Skip()
+
+import 'dart:async';
+
+import 'package:test/test.dart';
+
+void main() {
+  test("fail", () => throw 'oh no');
+}
+''');
+
+    var result = _runUnittest(["test.dart"]);
+    expect(result.stdout, contains("+0 ~1: All tests skipped."));
+  });
+
   group("flags:", () {
     test("with the --color flag, uses colors", () {
       new File(p.join(_sandbox, "test.dart")).writeAsStringSync(_failure);