From b868624b373f25140abbfeea940013214e462bfc Mon Sep 17 00:00:00 2001
From: Natalie Weizenbaum <nweiz@google.com>
Date: Mon, 23 Mar 2015 17:50:48 -0700
Subject: [PATCH] Add a function to parse metadata into a usable object.
See #6
R=rnystrom@google.com
Review URL: https://codereview.chromium.org//1008483003
---
lib/src/backend/metadata.dart | 16 +++
lib/src/frontend/test_on.dart | 17 +++
lib/src/runner/parse_metadata.dart | 110 ++++++++++++++++
lib/src/util/dart.dart | 10 --
pubspec.yaml | 1 +
test/runner/loader_test.dart | 2 +-
test/runner/parse_metadata_test.dart | 94 ++++++++++++++
test/util/parse_annotations_test.dart | 179 --------------------------
8 files changed, 239 insertions(+), 190 deletions(-)
create mode 100644 lib/src/backend/metadata.dart
create mode 100644 lib/src/frontend/test_on.dart
create mode 100644 lib/src/runner/parse_metadata.dart
create mode 100644 test/runner/parse_metadata_test.dart
delete mode 100644 test/util/parse_annotations_test.dart
diff --git a/lib/src/backend/metadata.dart b/lib/src/backend/metadata.dart
new file mode 100644
index 00000000..738a7cf5
--- /dev/null
+++ b/lib/src/backend/metadata.dart
@@ -0,0 +1,16 @@
+// 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 unittest.backend.metadata;
+
+/// 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;
+
+ Metadata(this.testOn);
+}
diff --git a/lib/src/frontend/test_on.dart b/lib/src/frontend/test_on.dart
new file mode 100644
index 00000000..d5ed9754
--- /dev/null
+++ b/lib/src/frontend/test_on.dart
@@ -0,0 +1,17 @@
+// 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 unittest.frontend.test_on;
+
+/// An annotation indicating which platforms a test or test suite supports.
+///
+/// For the full syntax of [expression], see [the README][readme].
+///
+/// [readme]: https://github.com/dart-lang/unittest/#readme
+class TestOn {
+ /// The expression specifying the platform.
+ final String expression;
+
+ const TestOn(this.expression);
+}
diff --git a/lib/src/runner/parse_metadata.dart b/lib/src/runner/parse_metadata.dart
new file mode 100644
index 00000000..5599922b
--- /dev/null
+++ b/lib/src/runner/parse_metadata.dart
@@ -0,0 +1,110 @@
+// 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 unittest.runner.parse_metadata;
+
+import 'dart:io';
+
+import 'package:analyzer/analyzer.dart';
+import 'package:analyzer/src/generated/ast.dart';
+import 'package:path/path.dart' as p;
+import 'package:source_span/source_span.dart';
+
+import '../backend/metadata.dart';
+import '../util/dart.dart';
+
+/// Parse the test metadata for the test file at [path].
+///
+/// Throws an [AnalysisError] if parsing fails or a [FormatException] if the
+/// test annotations are incorrect.
+Metadata parseMetadata(String path) {
+ var testOn;
+
+ var contents = new File(path).readAsStringSync();
+ var directives = parseDirectives(contents, name: path).directives;
+ var annotations = directives.isEmpty ? [] : directives.first.metadata;
+
+ // We explicitly *don't* just look for "package:unittest" imports here,
+ // because it could be re-exported from another library.
+ var prefixes = directives.map((directive) {
+ if (directive is! ImportDirective) return null;
+ if (directive.prefix == null) return null;
+ return directive.prefix.name;
+ }).where((prefix) => prefix != null).toSet();
+
+ for (var annotation in annotations) {
+ // The annotation syntax is ambiguous between named constructors and
+ // prefixed annotations, so we need to resolve that ambiguity using the
+ // known prefixes. The analyzer parses "@x.y()" as prefix "x", annotation
+ // "y", and named constructor null. It parses "@x.y.z()" as prefix "x",
+ // annotation "y", and named constructor "z".
+ var name;
+ var constructorName;
+ var identifier = annotation.name;
+ if (identifier is PrefixedIdentifier &&
+ !prefixes.contains(identifier.prefix.name) &&
+ annotation.constructorName == null) {
+ name = identifier.prefix.name;
+ constructorName = identifier.identifier.name;
+ } else {
+ name = identifier is PrefixedIdentifier
+ ? identifier.identifier.name
+ : identifier.name;
+ if (annotation.constructorName != null) {
+ constructorName = annotation.constructorName.name;
+ }
+ }
+
+ if (name != 'TestOn') continue;
+ if (constructorName != null) {
+ throw new SourceSpanFormatException(
+ 'TestOn doesn\'t have a constructor named "$constructorName".',
+ _spanFor(identifier.identifier, path));
+ }
+
+ if (annotation.arguments == null) {
+ throw new SourceSpanFormatException(
+ 'TestOn takes one argument.', _spanFor(annotation, path));
+ }
+
+ var args = annotation.arguments.arguments;
+ if (args.isEmpty) {
+ throw new SourceSpanFormatException(
+ 'TestOn takes one argument.', _spanFor(annotation.arguments, path));
+ }
+
+ if (args.first is NamedExpression) {
+ throw new SourceSpanFormatException(
+ "TestOn doesn't take named parameters.", _spanFor(args.first, path));
+ }
+
+ if (args.length > 1) {
+ throw new SourceSpanFormatException(
+ "TestOn takes only one argument.",
+ _spanFor(annotation.arguments, path));
+ }
+
+ if (args.first is! StringLiteral) {
+ throw new SourceSpanFormatException(
+ "TestOn takes a String.", _spanFor(args.first, path));
+ }
+
+ if (testOn != null) {
+ throw new SourceSpanFormatException(
+ "Only a single TestOn annotation may be used for a given test file.",
+ _spanFor(annotation, path));
+ }
+
+ testOn = args.first.stringValue;
+ }
+
+ return new Metadata(testOn);
+}
+
+/// Creates a [SourceSpan] for [node].
+SourceSpan _spanFor(AstNode node, String path) =>
+ // Load a SourceFile from scratch here since we're only ever going to emit
+ // one error per file anyway.
+ new SourceFile(new File(path).readAsStringSync(), url: p.toUri(path))
+ .span(node.offset, node.end);
diff --git a/lib/src/util/dart.dart b/lib/src/util/dart.dart
index 76f10b51..5cf02e53 100644
--- a/lib/src/util/dart.dart
+++ b/lib/src/util/dart.dart
@@ -71,13 +71,3 @@ void _isolateBuffer(message) {
});
});
}
-
-/// Parse all the annotations at the beginning of a Dart file.
-///
-/// This will parse annotations until the first non-annotation production is
-/// reached.
-List<Annotation> parseAnnotations(String path) {
- var contents = new File(path).readAsStringSync();
- var directives = parseDirectives(contents, name: path).directives;
- return directives.isEmpty ? [] : directives.first.metadata;
-}
diff --git a/pubspec.yaml b/pubspec.yaml
index 25215147..35be3283 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -13,6 +13,7 @@ dependencies:
shelf: '^0.5.3'
shelf_static: '^0.2.0'
shelf_web_socket: '^0.0.1'
+ source_span: '^1.0.0'
stack_trace: '^1.2.0'
# Use a tight version constraint to ensure that a constraint on unittest
diff --git a/test/runner/loader_test.dart b/test/runner/loader_test.dart
index c2213416..de83e820 100644
--- a/test/runner/loader_test.dart
+++ b/test/runner/loader_test.dart
@@ -6,7 +6,7 @@ import 'dart:io';
import 'package:path/path.dart' as p;
import 'package:unittest/src/backend/state.dart';
-import 'package:unittest/src/runner/test_platform.dart';
+import 'package:unittest/src/backend/test_platform.dart';
import 'package:unittest/src/runner/loader.dart';
import 'package:unittest/unittest.dart';
diff --git a/test/runner/parse_metadata_test.dart b/test/runner/parse_metadata_test.dart
new file mode 100644
index 00000000..5dfa6eaf
--- /dev/null
+++ b/test/runner/parse_metadata_test.dart
@@ -0,0 +1,94 @@
+// 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.
+
+import 'dart:io';
+
+import 'package:path/path.dart' as p;
+import 'package:unittest/unittest.dart';
+import 'package:unittest/src/runner/parse_metadata.dart';
+
+String _sandbox;
+String _path;
+
+void main() {
+ setUp(() {
+ _sandbox = Directory.systemTemp.createTempSync('unittest_').path;
+ _path = p.join(_sandbox, "test.dart");
+ });
+
+ tearDown(() {
+ new Directory(_sandbox).deleteSync(recursive: true);
+ });
+
+ test("returns empty metadata for an empty file", () {
+ new File(_path).writeAsStringSync("");
+ var metadata = parseMetadata(_path);
+ expect(metadata.testOn, isNull);
+ });
+
+ test("ignores irrelevant annotations", () {
+ new File(_path).writeAsStringSync("@Fblthp\n@Fblthp.foo\nlibrary foo;");
+ var metadata = parseMetadata(_path);
+ expect(metadata.testOn, isNull);
+ });
+
+ test("parses a valid annotation", () {
+ new File(_path).writeAsStringSync("@TestOn('foo')\nlibrary foo;");
+ var metadata = parseMetadata(_path);
+ expect(metadata.testOn, equals("foo"));
+ });
+
+ test("parses a prefixed annotation", () {
+ new File(_path).writeAsStringSync(
+ "@foo.TestOn('foo')\n"
+ "import 'package:unittest/unittest.dart' as foo;");
+ var metadata = parseMetadata(_path);
+ expect(metadata.testOn, equals("foo"));
+ });
+
+ test("ignores a constructor named TestOn", () {
+ new File(_path).writeAsStringSync("@foo.TestOn('foo')\nlibrary foo;");
+ var metadata = parseMetadata(_path);
+ expect(metadata.testOn, isNull);
+ });
+
+ group("throws an error for", () {
+ test("a named constructor", () {
+ new File(_path).writeAsStringSync("@TestOn.name('foo')\nlibrary foo;");
+ expect(() => parseMetadata(_path), throwsFormatException);
+ });
+
+ test("no argument list", () {
+ new File(_path).writeAsStringSync("@TestOn\nlibrary foo;");
+ expect(() => parseMetadata(_path), throwsFormatException);
+ });
+
+ test("an empty argument list", () {
+ new File(_path).writeAsStringSync("@TestOn()\nlibrary foo;");
+ expect(() => parseMetadata(_path), throwsFormatException);
+ });
+
+ test("a named argument", () {
+ new File(_path).writeAsStringSync(
+ "@TestOn(expression: 'foo')\nlibrary foo;");
+ expect(() => parseMetadata(_path), throwsFormatException);
+ });
+
+ test("multiple arguments", () {
+ new File(_path).writeAsStringSync("@TestOn('foo', 'bar')\nlibrary foo;");
+ expect(() => parseMetadata(_path), throwsFormatException);
+ });
+
+ test("a non-string argument", () {
+ new File(_path).writeAsStringSync("@TestOn(123)\nlibrary foo;");
+ expect(() => parseMetadata(_path), throwsFormatException);
+ });
+
+ test("multiple @TestOns", () {
+ new File(_path).writeAsStringSync(
+ "@TestOn('foo')\n@TestOn('bar')\nlibrary foo;");
+ expect(() => parseMetadata(_path), throwsFormatException);
+ });
+ });
+}
\ No newline at end of file
diff --git a/test/util/parse_annotations_test.dart b/test/util/parse_annotations_test.dart
deleted file mode 100644
index a655d160..00000000
--- a/test/util/parse_annotations_test.dart
+++ /dev/null
@@ -1,179 +0,0 @@
-// 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.
-
-import 'dart:io';
-
-import 'package:analyzer/src/generated/ast.dart';
-import 'package:path/path.dart' as p;
-import 'package:unittest/unittest.dart';
-import 'package:unittest/src/util/dart.dart';
-
-String _sandbox;
-String _path;
-
-void main() {
- setUp(() {
- _sandbox = Directory.systemTemp.createTempSync('unittest_').path;
- _path = p.join(_sandbox, "test.dart");
- });
-
- tearDown(() {
- new Directory(_sandbox).deleteSync(recursive: true);
- });
-
- test("with an empty file", () {
- new File(_path).writeAsStringSync("");
-
- var annotations = parseAnnotations(_path);
- expect(annotations, isEmpty);
- });
-
- test("before a library tag", () {
- new File(_path).writeAsStringSync("@Annotation()\nlibrary foo;");
-
- var annotations = parseAnnotations(_path);
- expect(annotations, hasLength(1));
- expect(annotations.first.name.name, equals('Annotation'));
- expect(annotations.first.arguments.arguments, isEmpty);
- });
-
- test("before an import", () {
- new File(_path).writeAsStringSync("@Annotation()\nimport 'foo';");
-
- var annotations = parseAnnotations(_path);
- expect(annotations, hasLength(1));
- expect(annotations.first.name.name, equals('Annotation'));
- expect(annotations.first.arguments.arguments, isEmpty);
- });
-
- test("multiple annotations", () {
- new File(_path).writeAsStringSync(
- "@Annotation1() @Annotation2()\n@Annotation3()\nlibrary foo;");
-
- var annotations = parseAnnotations(_path);
- expect(annotations, hasLength(3));
- expect(annotations[0].name.name, equals('Annotation1'));
- expect(annotations[0].arguments.arguments, isEmpty);
- expect(annotations[1].name.name, equals('Annotation2'));
- expect(annotations[1].arguments.arguments, isEmpty);
- expect(annotations[2].name.name, equals('Annotation3'));
- expect(annotations[2].arguments.arguments, isEmpty);
- });
-
- test("with no arguments", () {
- new File(_path).writeAsStringSync("@Annotation\nlibrary foo;");
-
- var annotations = parseAnnotations(_path);
- expect(annotations, hasLength(1));
- expect(annotations.first.name.name, equals('Annotation'));
- expect(annotations.first.arguments, isNull);
- });
-
- test("with positional arguments", () {
- new File(_path).writeAsStringSync("@Annotation('foo', 12)\nlibrary foo;");
-
- var annotations = parseAnnotations(_path);
- expect(annotations, hasLength(1));
- expect(annotations.first.name.name, equals('Annotation'));
- var args = annotations.first.arguments.arguments;
- expect(args, hasLength(2));
- expect(args[0], new isInstanceOf<StringLiteral>());
- expect(args[0].stringValue, equals('foo'));
- expect(args[1], new isInstanceOf<IntegerLiteral>());
- expect(args[1].value, equals(12));
- });
-
- test("with named arguments", () {
- new File(_path).writeAsStringSync(
- "@Annotation(name1: 'foo', name2: 12)\nlibrary foo;");
-
- var annotations = parseAnnotations(_path);
- expect(annotations, hasLength(1));
- expect(annotations.first.name.name, equals('Annotation'));
- var args = annotations.first.arguments.arguments;
- expect(args, hasLength(2));
- expect(args[0], new isInstanceOf<NamedExpression>());
- expect(args[0].expression, new isInstanceOf<StringLiteral>());
- expect(args[0].expression.stringValue, equals('foo'));
- expect(args[1], new isInstanceOf<NamedExpression>());
- expect(args[1].expression, new isInstanceOf<IntegerLiteral>());
- expect(args[1].expression.value, equals(12));
- });
-
- test("with a prefix/named constructor", () {
- new File(_path).writeAsStringSync("@Annotation.name()\nlibrary foo;");
-
- var annotations = parseAnnotations(_path);
- expect(annotations, hasLength(1));
- expect(annotations.first.name, new isInstanceOf<PrefixedIdentifier>());
- expect(annotations.first.name.prefix.name, equals('Annotation'));
- expect(annotations.first.name.identifier.name, equals('name'));
- expect(annotations.first.constructorName, isNull);
- expect(annotations.first.arguments.arguments, isEmpty);
- });
-
- test("with a prefix and named constructor", () {
- new File(_path).writeAsStringSync(
- "@prefix.Annotation.name()\nlibrary foo;");
-
- var annotations = parseAnnotations(_path);
- expect(annotations, hasLength(1));
- expect(annotations.first.name, new isInstanceOf<PrefixedIdentifier>());
- expect(annotations.first.name.prefix.name, equals('prefix'));
- expect(annotations.first.name.identifier.name, equals('Annotation'));
- expect(annotations.first.constructorName.name, equals('name'));
- expect(annotations.first.arguments.arguments, isEmpty);
- });
-
- test("annotations after the first directive are ignored", () {
- new File(_path).writeAsStringSync(
- "library foo;\n@prefix.Annotation.name()");
-
- expect(parseAnnotations(_path), isEmpty);
- });
-
- group("comments are ignored", () {
- test("before an annotation", () {
- new File(_path).writeAsStringSync(
- "/* comment */@Annotation()\nlibrary foo;");
-
- var annotations = parseAnnotations(_path);
- expect(annotations, hasLength(1));
- expect(annotations.first.name.name, equals('Annotation'));
- expect(annotations.first.arguments.arguments, isEmpty);
- });
-
- test("within an annotation", () {
- new File(_path).writeAsStringSync(
- "@Annotation(/* comment */)\nlibrary foo;");
-
- var annotations = parseAnnotations(_path);
- expect(annotations, hasLength(1));
- expect(annotations.first.name.name, equals('Annotation'));
- expect(annotations.first.arguments.arguments, isEmpty);
- });
-
- test("after an annotation", () {
- new File(_path).writeAsStringSync(
- "@Annotation()/* comment */\nlibrary foo;");
-
- var annotations = parseAnnotations(_path);
- expect(annotations, hasLength(1));
- expect(annotations.first.name.name, equals('Annotation'));
- expect(annotations.first.arguments.arguments, isEmpty);
- });
-
- test("between annotations", () {
- new File(_path).writeAsStringSync(
- "@Annotation1()/* comment */@Annotation2()\nlibrary foo;");
-
- var annotations = parseAnnotations(_path);
- expect(annotations, hasLength(2));
- expect(annotations[0].name.name, equals('Annotation1'));
- expect(annotations[0].arguments.arguments, isEmpty);
- expect(annotations[1].name.name, equals('Annotation2'));
- expect(annotations[1].arguments.arguments, isEmpty);
- });
- });
-}
--
GitLab