From dfa438a001e743b08b30e3581cabcb7904f40d23 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald <jakemac@google.com> Date: Tue, 9 Apr 2019 09:26:45 -0700 Subject: [PATCH] update parseMetadata to take a required contents argument (#1017) --- .../test/test/runner/parse_metadata_test.dart | 202 ++++++++++-------- pkgs/test_core/lib/src/runner/loader.dart | 3 +- .../lib/src/runner/parse_metadata.dart | 18 +- 3 files changed, 115 insertions(+), 108 deletions(-) diff --git a/pkgs/test/test/runner/parse_metadata_test.dart b/pkgs/test/test/runner/parse_metadata_test.dart index 21066c1d..aa10b7cd 100644 --- a/pkgs/test/test/runner/parse_metadata_test.dart +++ b/pkgs/test/test/runner/parse_metadata_test.dart @@ -3,77 +3,65 @@ // BSD-style license that can be found in the LICENSE file. @TestOn('vm') -import 'dart:io'; - -import 'package:path/path.dart' as p; import 'package:test/test.dart'; import 'package:test_api/src/backend/platform_selector.dart'; import 'package:test_api/src/backend/runtime.dart'; import 'package:test_api/src/backend/suite_platform.dart'; import 'package:test_core/src/runner/parse_metadata.dart'; -import 'package:test_core/src/util/io.dart'; - -String _sandbox; -String _path; +final _path = 'test.dart'; void main() { - setUp(() { - _sandbox = createTempDir(); - _path = p.join(_sandbox, 'test.dart'); - }); - - tearDown(() { - Directory(_sandbox).deleteSync(recursive: true); - }); - test('returns empty metadata for an empty file', () { - File(_path).writeAsStringSync(''); - var metadata = parseMetadata(_path, Set()); + var metadata = parseMetadata(_path, '', Set()); expect(metadata.testOn, equals(PlatformSelector.all)); expect(metadata.timeout.scaleFactor, equals(1)); }); test('ignores irrelevant annotations', () { - File(_path).writeAsStringSync('@Fblthp\n@Fblthp.foo\nlibrary foo;'); - var metadata = parseMetadata(_path, Set()); + var metadata = + parseMetadata(_path, '@Fblthp\n@Fblthp.foo\nlibrary foo;', Set()); expect(metadata.testOn, equals(PlatformSelector.all)); }); test('parses a prefixed annotation', () { - File(_path).writeAsStringSync("@foo.TestOn('vm')\n" - "import 'package:test/test.dart' as foo;"); - var metadata = parseMetadata(_path, Set()); + var metadata = parseMetadata( + _path, + "@foo.TestOn('vm')\n" + "import 'package:test/test.dart' as foo;", + Set()); expect(metadata.testOn.evaluate(SuitePlatform(Runtime.vm)), isTrue); expect(metadata.testOn.evaluate(SuitePlatform(Runtime.chrome)), isFalse); }); group('@TestOn:', () { test('parses a valid annotation', () { - File(_path).writeAsStringSync("@TestOn('vm')\nlibrary foo;"); - var metadata = parseMetadata(_path, Set()); + var metadata = parseMetadata(_path, "@TestOn('vm')\nlibrary foo;", Set()); expect(metadata.testOn.evaluate(SuitePlatform(Runtime.vm)), isTrue); expect(metadata.testOn.evaluate(SuitePlatform(Runtime.chrome)), isFalse); }); test('ignores a constructor named TestOn', () { - File(_path).writeAsStringSync("@foo.TestOn('foo')\nlibrary foo;"); - var metadata = parseMetadata(_path, Set()); + var metadata = + parseMetadata(_path, "@foo.TestOn('foo')\nlibrary foo;", Set()); expect(metadata.testOn, equals(PlatformSelector.all)); }); group('throws an error for', () { test('multiple @TestOns', () { - File(_path) - .writeAsStringSync("@TestOn('foo')\n@TestOn('bar')\nlibrary foo;"); - expect(() => parseMetadata(_path, Set()), throwsFormatException); + expect( + () => parseMetadata( + _path, "@TestOn('foo')\n@TestOn('bar')\nlibrary foo;", Set()), + throwsFormatException); }); }); }); group('@Timeout:', () { test('parses a valid duration annotation', () { - File(_path).writeAsStringSync(''' + var metadata = parseMetadata( + _path, + ''' @Timeout(const Duration( hours: 1, minutes: 2, @@ -82,8 +70,8 @@ void main() { microseconds: 5)) library foo; -'''); - var metadata = parseMetadata(_path, Set()); +''', + Set()); expect( metadata.timeout.duration, equals(Duration( @@ -95,7 +83,9 @@ library foo; }); test('parses a valid duration omitting const', () { - File(_path).writeAsStringSync(''' + var metadata = parseMetadata( + _path, + ''' @Timeout(Duration( hours: 1, minutes: 2, @@ -104,8 +94,8 @@ library foo; microseconds: 5)) library foo; -'''); - var metadata = parseMetadata(_path, Set()); +''', + Set()); expect( metadata.timeout.duration, equals(Duration( @@ -117,7 +107,9 @@ library foo; }); test('parses a valid duration with an import prefix', () { - File(_path).writeAsStringSync(''' + var metadata = parseMetadata( + _path, + ''' @Timeout(core.Duration( hours: 1, minutes: 2, @@ -125,8 +117,8 @@ library foo; milliseconds: 4, microseconds: 5)) import 'dart:core' as core; -'''); - var metadata = parseMetadata(_path, Set()); +''', + Set()); expect( metadata.timeout.duration, equals(Duration( @@ -138,126 +130,138 @@ import 'dart:core' as core; }); test('parses a valid int factor annotation', () { - File(_path).writeAsStringSync(''' + var metadata = parseMetadata( + _path, + ''' @Timeout.factor(1) library foo; -'''); - var metadata = parseMetadata(_path, Set()); +''', + Set()); expect(metadata.timeout.scaleFactor, equals(1)); }); test('parses a valid int factor annotation with an import prefix', () { - File(_path).writeAsStringSync(''' + var metadata = parseMetadata( + _path, + ''' @test.Timeout.factor(1) import 'package:test/test.dart' as test; -'''); - var metadata = parseMetadata(_path, Set()); +''', + Set()); expect(metadata.timeout.scaleFactor, equals(1)); }); test('parses a valid double factor annotation', () { - File(_path).writeAsStringSync(''' + var metadata = parseMetadata( + _path, + ''' @Timeout.factor(0.5) library foo; -'''); - var metadata = parseMetadata(_path, Set()); +''', + Set()); expect(metadata.timeout.scaleFactor, equals(0.5)); }); test('parses a valid Timeout.none annotation', () { - File(_path).writeAsStringSync(''' + var metadata = parseMetadata( + _path, + ''' @Timeout.none library foo; -'''); - var metadata = parseMetadata(_path, Set()); +''', + Set()); expect(metadata.timeout, same(Timeout.none)); }); test('ignores a constructor named Timeout', () { - File(_path).writeAsStringSync("@foo.Timeout('foo')\nlibrary foo;"); - var metadata = parseMetadata(_path, Set()); + var metadata = + parseMetadata(_path, "@foo.Timeout('foo')\nlibrary foo;", Set()); expect(metadata.timeout.scaleFactor, equals(1)); }); group('throws an error for', () { test('multiple @Timeouts', () { - File(_path).writeAsStringSync( - '@Timeout.factor(1)\n@Timeout.factor(2)\nlibrary foo;'); - expect(() => parseMetadata(_path, Set()), throwsFormatException); + expect( + () => parseMetadata(_path, + '@Timeout.factor(1)\n@Timeout.factor(2)\nlibrary foo;', Set()), + throwsFormatException); }); }); }); group('@Skip:', () { test('parses a valid annotation', () { - File(_path).writeAsStringSync('@Skip()\nlibrary foo;'); - var metadata = parseMetadata(_path, Set()); + var metadata = parseMetadata(_path, '@Skip()\nlibrary foo;', Set()); expect(metadata.skip, isTrue); expect(metadata.skipReason, isNull); }); test('parses a valid annotation with a reason', () { - File(_path).writeAsStringSync("@Skip('reason')\nlibrary foo;"); - var metadata = parseMetadata(_path, Set()); + var metadata = + parseMetadata(_path, "@Skip('reason')\nlibrary foo;", Set()); expect(metadata.skip, isTrue); expect(metadata.skipReason, equals('reason')); }); test('ignores a constructor named Skip', () { - File(_path).writeAsStringSync("@foo.Skip('foo')\nlibrary foo;"); - var metadata = parseMetadata(_path, Set()); + var metadata = + parseMetadata(_path, "@foo.Skip('foo')\nlibrary foo;", Set()); expect(metadata.skip, isFalse); }); group('throws an error for', () { test('multiple @Skips', () { - File(_path) - .writeAsStringSync("@Skip('foo')\n@Skip('bar')\nlibrary foo;"); - expect(() => parseMetadata(_path, Set()), throwsFormatException); + expect( + () => parseMetadata( + _path, "@Skip('foo')\n@Skip('bar')\nlibrary foo;", Set()), + throwsFormatException); }); }); }); group('@Tags:', () { test('parses a valid annotation', () { - File(_path).writeAsStringSync("@Tags(['a'])\nlibrary foo;"); - var metadata = parseMetadata(_path, Set()); + var metadata = parseMetadata(_path, "@Tags(['a'])\nlibrary foo;", Set()); expect(metadata.tags, equals(['a'])); }); test('ignores a constructor named Tags', () { - File(_path).writeAsStringSync("@foo.Tags(['a'])\nlibrary foo;"); - var metadata = parseMetadata(_path, Set()); + var metadata = + parseMetadata(_path, "@foo.Tags(['a'])\nlibrary foo;", Set()); expect(metadata.tags, isEmpty); }); group('throws an error for', () { test('multiple @Tags', () { - File(_path) - .writeAsStringSync("@Tags(['a'])\n@Tags(['b'])\nlibrary foo;"); - expect(() => parseMetadata(_path, Set()), throwsFormatException); + expect( + () => parseMetadata( + _path, "@Tags(['a'])\n@Tags(['b'])\nlibrary foo;", Set()), + throwsFormatException); }); test('String interpolation', () { - File(_path) - .writeAsStringSync("@Tags(['\$a'])\nlibrary foo;\nconst a = 'a';"); - expect(() => parseMetadata(_path, Set()), throwsFormatException); + expect( + () => parseMetadata( + _path, "@Tags(['\$a'])\nlibrary foo;\nconst a = 'a';", Set()), + throwsFormatException); }); }); }); group('@OnPlatform:', () { test('parses a valid annotation', () { - File(_path).writeAsStringSync(''' + var metadata = parseMetadata( + _path, + ''' @OnPlatform({ 'chrome': Timeout.factor(2), 'vm': [Skip(), Timeout.factor(3)] }) -library foo;'''); - var metadata = parseMetadata(_path, Set()); +library foo;''', + Set()); var key = metadata.onPlatform.keys.first; expect(key.evaluate(SuitePlatform(Runtime.chrome)), isTrue); @@ -274,14 +278,16 @@ library foo;'''); }); test('parses a valid annotation with an import prefix', () { - File(_path).writeAsStringSync(''' + var metadata = parseMetadata( + _path, + ''' @test.OnPlatform({ 'chrome': test.Timeout.factor(2), 'vm': [test.Skip(), test.Timeout.factor(3)] }) import 'package:test/test.dart' as test; -'''); - var metadata = parseMetadata(_path, Set()); +''', + Set()); var key = metadata.onPlatform.keys.first; expect(key.evaluate(SuitePlatform(Runtime.chrome)), isTrue); @@ -298,34 +304,40 @@ import 'package:test/test.dart' as test; }); test('ignores a constructor named OnPlatform', () { - File(_path).writeAsStringSync("@foo.OnPlatform('foo')\nlibrary foo;"); - var metadata = parseMetadata(_path, Set()); + var metadata = + parseMetadata(_path, "@foo.OnPlatform('foo')\nlibrary foo;", Set()); expect(metadata.testOn, equals(PlatformSelector.all)); }); group('throws an error for', () { test('a map with a unparseable key', () { - File(_path).writeAsStringSync( - "@OnPlatform({'invalid': Skip()})\nlibrary foo;"); - expect(() => parseMetadata(_path, Set()), throwsFormatException); + expect( + () => parseMetadata( + _path, "@OnPlatform({'invalid': Skip()})\nlibrary foo;", Set()), + throwsFormatException); }); test("a map with an invalid value", () { - File(_path).writeAsStringSync( - "@OnPlatform({'vm': const TestOn('vm')})\nlibrary foo;"); - expect(() => parseMetadata(_path, Set()), throwsFormatException); + expect( + () => parseMetadata(_path, + "@OnPlatform({'vm': const TestOn('vm')})\nlibrary foo;", Set()), + throwsFormatException); }); test('a map with an invalid value in a list', () { - File(_path).writeAsStringSync( - "@OnPlatform({'vm': [const TestOn('vm')]})\nlibrary foo;"); - expect(() => parseMetadata(_path, Set()), throwsFormatException); + expect( + () => parseMetadata( + _path, + "@OnPlatform({'vm': [const TestOn('vm')]})\nlibrary foo;", + Set()), + throwsFormatException); }); test('multiple @OnPlatforms', () { - File(_path).writeAsStringSync( - '@OnPlatform({})\n@OnPlatform({})\nlibrary foo;'); - expect(() => parseMetadata(_path, Set()), throwsFormatException); + expect( + () => parseMetadata( + _path, '@OnPlatform({})\n@OnPlatform({})\nlibrary foo;', Set()), + throwsFormatException); }); }); }); diff --git a/pkgs/test_core/lib/src/runner/loader.dart b/pkgs/test_core/lib/src/runner/loader.dart index 1ea3693c..74fe5f29 100644 --- a/pkgs/test_core/lib/src/runner/loader.dart +++ b/pkgs/test_core/lib/src/runner/loader.dart @@ -168,7 +168,8 @@ class Loader { String path, SuiteConfiguration suiteConfig) async* { try { suiteConfig = suiteConfig.merge(SuiteConfiguration.fromMetadata( - parseMetadata(path, _runtimeVariables.toSet()))); + parseMetadata( + path, File(path).readAsStringSync(), _runtimeVariables.toSet()))); } on AnalyzerErrorGroup catch (_) { // Ignore the analyzer's error, since its formatting is much worse than // the VM's or dart2js's. diff --git a/pkgs/test_core/lib/src/runner/parse_metadata.dart b/pkgs/test_core/lib/src/runner/parse_metadata.dart index 7654de58..90946a17 100644 --- a/pkgs/test_core/lib/src/runner/parse_metadata.dart +++ b/pkgs/test_core/lib/src/runner/parse_metadata.dart @@ -2,8 +2,6 @@ // 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'; - // ignore: deprecated_member_use import 'package:analyzer/analyzer.dart'; import 'package:analyzer/dart/ast/ast.dart'; @@ -16,7 +14,7 @@ import 'package:test_api/src/frontend/timeout.dart'; // ignore: implementation_i import 'package:test_api/src/utils.dart'; // ignore: implementation_imports import '../util/dart.dart'; -/// Parse the test metadata for the test file at [path]. +/// Parse the test metadata for the test file at [path] with [contents]. /// /// The [platformVariables] are the set of variables that are valid for platform /// selectors in suite metadata, in addition to the built-in variables that are @@ -24,11 +22,9 @@ import '../util/dart.dart'; /// /// Throws an [AnalysisError] if parsing fails or a [FormatException] if the /// test annotations are incorrect. -/// -/// If [contents] is `null` then it will be read as a [File] using [path]. -Metadata parseMetadata(String path, Set<String> platformVariables, - {String contents}) => - _Parser(path, platformVariables, contents: contents).parse(); +Metadata parseMetadata( + String path, String contents, Set<String> platformVariables) => + _Parser(path, contents, platformVariables).parse(); /// A parser for test suite metadata. class _Parser { @@ -48,10 +44,8 @@ class _Parser { /// The actual contents of the file. String _contents; - _Parser(String path, this._platformVariables, {String contents}) - : _path = path, - _contents = contents ?? File(path).readAsStringSync() { - var directives = parseDirectives(_contents, name: path).directives; + _Parser(this._path, this._contents, this._platformVariables) { + var directives = parseDirectives(_contents, name: _path).directives; _annotations = directives.isEmpty ? [] : directives.first.metadata; // We explicitly *don't* just look for "package:test" imports here, -- GitLab