From 4a65a977a89b4333310e9eb29dbe8eec296338c2 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 9 Apr 2025 18:02:22 +0100 Subject: [PATCH 01/13] Allow specifying an explicit location for test/groups Usually test locations (which are used both to report the location of tests in the json reporter, and also for filtering tests by line/col) are inferred from the call stack when test() is called. This changes adds a new `location` parameter to group/test to allow this information to be provided explicitly. This can be useful where the call to `test()` doesn't contain the actual location of the declaration in the call stack (for example when using `pkg:test_reflective_loader`). See https://github.com/dart-lang/test/issues/2029 See https://github.com/Dart-Code/Dart-Code/issues/5480 --- pkgs/test/test/runner/json_reporter_test.dart | 64 +++++++++++++++ .../test/test/runner/json_reporter_utils.dart | 9 ++- pkgs/test/test/runner/line_and_col_test.dart | 78 ++++++++++++++++++- pkgs/test_api/lib/backend.dart | 1 + pkgs/test_api/lib/scaffolding.dart | 1 + pkgs/test_api/lib/src/backend/declarer.dart | 39 ++++++++-- pkgs/test_api/lib/src/backend/group.dart | 12 ++- .../test_api/lib/src/backend/group_entry.dart | 11 +++ pkgs/test_api/lib/src/backend/invoker.dart | 15 +++- .../lib/src/backend/remote_listener.dart | 2 + .../lib/src/backend/test_location.dart | 27 +++++++ .../lib/src/scaffolding/test_structure.dart | 12 ++- pkgs/test_core/lib/src/runner.dart | 19 ++++- .../src/runner/plugin/platform_helpers.dart | 20 +++-- .../lib/src/runner/reporter/json.dart | 27 ++++++- .../test_core/lib/src/runner/runner_test.dart | 10 ++- pkgs/test_core/lib/src/scaffolding.dart | 11 ++- 17 files changed, 322 insertions(+), 36 deletions(-) create mode 100644 pkgs/test_api/lib/src/backend/test_location.dart diff --git a/pkgs/test/test/runner/json_reporter_test.dart b/pkgs/test/test/runner/json_reporter_test.dart index a771438ef..b6a6a64cb 100644 --- a/pkgs/test/test/runner/json_reporter_test.dart +++ b/pkgs/test/test/runner/json_reporter_test.dart @@ -587,6 +587,70 @@ void customTest(String name, dynamic Function() testFn) => test(name, testFn); ''', }); }); + + test('groups and tests with custom locations', () { + return _expectReport(''' + group('group 1 inferred', () { + setUpAll(() {}); + test('test 1 inferred', () {}); + tearDownAll(() {}); + }); + group('group 2 custom', location: TestLocation('file:///foo/group', 123, 234), () { + setUpAll(location: TestLocation('file:///foo/setUpAll', 345, 456), () {}); + test('test 2 custom', location: TestLocation('file:///foo/test', 567, 789), () {}); + tearDownAll(location: TestLocation('file:///foo/tearDownAll', 890, 901), () {}); + }); + ''', [ + [ + suiteJson(0), + testStartJson(1, 'loading test.dart', groupIDs: []), + testDoneJson(1, hidden: true), + ], + [ + groupJson(2, testCount: 2), + groupJson(3, + name: 'group 1 inferred', + parentID: 2, + line: 6, + column: 7, + testCount: 1), + testStartJson(4, 'group 1 inferred (setUpAll)', + groupIDs: [2, 3], line: 7, column: 9), + testDoneJson(4, hidden: true), + testStartJson(5, 'group 1 inferred test 1 inferred', + groupIDs: [2, 3], line: 8, column: 9), + testDoneJson(5), + testStartJson(6, 'group 1 inferred (tearDownAll)', + groupIDs: [2, 3], line: 9, column: 9), + testDoneJson(6, hidden: true), + groupJson(7, + name: 'group 2 custom', + parentID: 2, + url: 'file:///foo/group', + line: 123, + column: 234, + testCount: 1), + testStartJson(8, 'group 2 custom (setUpAll)', + url: 'file:///foo/setUpAll', + groupIDs: [2, 7], + line: 345, + column: 456), + testDoneJson(8, hidden: true), + testStartJson(9, 'group 2 custom test 2 custom', + url: 'file:///foo/test', + groupIDs: [2, 7], + line: 567, + column: 789), + testDoneJson(9), + testStartJson(10, 'group 2 custom (tearDownAll)', + url: 'file:///foo/tearDownAll', + groupIDs: [2, 7], + line: 890, + column: 901), + testDoneJson(10, hidden: true), + ] + ], doneJson()); + }); }); test( diff --git a/pkgs/test/test/runner/json_reporter_utils.dart b/pkgs/test/test/runner/json_reporter_utils.dart index bbdbb78f0..3c921b005 100644 --- a/pkgs/test/test/runner/json_reporter_utils.dart +++ b/pkgs/test/test/runner/json_reporter_utils.dart @@ -89,6 +89,7 @@ Map groupJson(int id, int? parentID, Object? skip, int? testCount, + String? url, int? line, int? column}) { if ((line == null) != (column == null)) { @@ -96,6 +97,8 @@ Map groupJson(int id, 'line and column must either both be null or both be passed'); } + url ??= + line == null ? null : p.toUri(p.join(d.sandbox, 'test.dart')).toString(); return { 'type': 'group', 'group': { @@ -107,9 +110,7 @@ Map groupJson(int id, 'testCount': testCount ?? 1, 'line': line, 'column': column, - 'url': line == null - ? null - : p.toUri(p.join(d.sandbox, 'test.dart')).toString() + 'url': url } }; } @@ -117,7 +118,7 @@ Map groupJson(int id, /// Returns the event emitted by the JSON reporter indicating that a test has /// begun running. /// -/// If [parentIDs] is passed, it's the IDs of groups containing this test. If +/// If [groupIDs] is passed, it's the IDs of groups containing this test. If /// [skip] is `true`, the test is expected to be marked as skipped without a /// reason. If it's a [String], the test is expected to be marked as skipped /// with that reason. diff --git a/pkgs/test/test/runner/line_and_col_test.dart b/pkgs/test/test/runner/line_and_col_test.dart index c513cefc6..501600d7d 100644 --- a/pkgs/test/test/runner/line_and_col_test.dart +++ b/pkgs/test/test/runner/line_and_col_test.dart @@ -56,13 +56,43 @@ void main() { await test.shouldExit(0); }); + test('additionally selects test with matching custom location', () async { + var testFileUri = Uri.file(d.file('test.dart').io.path); + var notTestFileUri = Uri.file(d.file('not_test.dart').io.path); + await d.file('test.dart', ''' + import 'package:test/test.dart'; + + void main() { + test("a", () {}); // Line 4 from stack trace + + // Custom line 4 match + test("b", location: TestLocation('$testFileUri', 4, 0), () {}); + + // Custom different line, same file + test("c", location: TestLocation('$testFileUri', 5, 0), () => throw TestFailure("oh no")); + + // Custom line 4 match but different file (no match) + test("d", location: TestLocation('$notTestFileUri', 4, 0), () => throw TestFailure("oh no")); + } + ''').create(); + + var test = await runTest(['test.dart?line=4']); + + expect( + test.stdout, + emitsThrough(contains('+2: All tests passed!')), + ); + + await test.shouldExit(0); + }); + test('selects groups with a matching line', () async { await d.file('test.dart', ''' import 'package:test/test.dart'; void main() { group("a", () { - test("b", () {}); + test("a", () {}); }); group("b", () { test("b", () => throw TestFailure("oh no")); @@ -80,6 +110,52 @@ void main() { await test.shouldExit(0); }); + test('additionally selects groups with a matching custom location', + () async { + // TODO(dantup): This fails because we don't ever look at a groups + // location.. instead, we only look at tests. The reason we can + // normally run groups by line/col is because they just happen to be + // in the stack trace for the test() call too. + // + // So maybe we need to look up the tree to match location (in + // Runner._loadSuites() filter)? + var testFileUri = Uri.file(d.file('test.dart').io.path); + var notTestFileUri = Uri.file(d.file('not_test.dart').io.path); + await d.file('test.dart', ''' + import 'package:test/test.dart'; + + void main() { + group("a", () { // Line 4 from stack trace + test("a", () {}); + }); + + // Custom line 4 match + group("b", location: TestLocation('$testFileUri', 4, 0), () { + test("b", () {}); + }); + + // Custom different line, same file + group("c", location: TestLocation('$testFileUri', 5, 0), () { + test("c", () => throw TestFailure("oh no")); + }); + + // Custom line 4 match but different file (no match) + group("d", location: TestLocation('$notTestFileUri', 4, 0), () { + test("d", () => throw TestFailure("oh no")); + }); + } + ''').create(); + + var test = await runTest(['test.dart?line=4']); + + expect( + test.stdout, + emitsThrough(contains('+2: All tests passed!')), + ); + + await test.shouldExit(0); + }); + test('No matching tests', () async { await d.file('test.dart', ''' import 'package:test/test.dart'; diff --git a/pkgs/test_api/lib/backend.dart b/pkgs/test_api/lib/backend.dart index 5da47b6c3..9289e3d39 100644 --- a/pkgs/test_api/lib/backend.dart +++ b/pkgs/test_api/lib/backend.dart @@ -11,3 +11,4 @@ export 'src/backend/runtime.dart' show Runtime; export 'src/backend/stack_trace_formatter.dart' show StackTraceFormatter; export 'src/backend/stack_trace_mapper.dart' show StackTraceMapper; export 'src/backend/suite_platform.dart' show SuitePlatform; +export 'src/backend/test_location.dart' show TestLocation; diff --git a/pkgs/test_api/lib/scaffolding.dart b/pkgs/test_api/lib/scaffolding.dart index 9d864bbb9..979337767 100644 --- a/pkgs/test_api/lib/scaffolding.dart +++ b/pkgs/test_api/lib/scaffolding.dart @@ -16,6 +16,7 @@ export 'src/backend/configuration/skip.dart' show Skip; export 'src/backend/configuration/tags.dart' show Tags; export 'src/backend/configuration/test_on.dart' show TestOn; export 'src/backend/configuration/timeout.dart' show Timeout; +export 'src/backend/test_location.dart' show TestLocation; export 'src/scaffolding/spawn_hybrid.dart' show spawnHybridCode, spawnHybridUri; export 'src/scaffolding/test_structure.dart' show addTearDown, group, setUp, setUpAll, tearDown, tearDownAll, test; diff --git a/pkgs/test_api/lib/src/backend/declarer.dart b/pkgs/test_api/lib/src/backend/declarer.dart index 76f7563a7..4e9818d1e 100644 --- a/pkgs/test_api/lib/src/backend/declarer.dart +++ b/pkgs/test_api/lib/src/backend/declarer.dart @@ -13,6 +13,7 @@ import 'group_entry.dart'; import 'invoker.dart'; import 'metadata.dart'; import 'test.dart'; +import 'test_location.dart'; /// A class that manages the state of tests as they're declared. /// @@ -44,6 +45,9 @@ class Declarer { /// This is `null` for the root (implicit) group. final Trace? _trace; + /// The optional location override for this group. + final TestLocation? _location; + /// Whether to collect stack traces for [GroupEntry]s. final bool _collectTraces; @@ -69,6 +73,9 @@ class Declarer { /// [setUpAll] is always run and the rest are only run if that one succeeds. Trace? _setUpAllTrace; + /// The optional location override for [setUpAll]. + TestLocation? _setUpAllLocation; + /// The tear-down functions to run once for this group. final _tearDownAlls = []; @@ -78,6 +85,9 @@ class Declarer { /// one trace. The first trace matches [_setUpAllTrace]. Trace? _tearDownAllTrace; + /// The optional location override for [tearDownAll]. + TestLocation? _tearDownAllLocation; + /// The children of this group, either tests or sub-groups. /// /// All modifications to this must go through [_addEntry]. @@ -157,6 +167,7 @@ class Declarer { platformVariables ?? const UnmodifiableSetView.empty(), collectTraces, null, + null, noRetry, fullTestName, allowDuplicateTestNames ? null : {}, @@ -170,6 +181,7 @@ class Declarer { this._platformVariables, this._collectTraces, this._trace, + this._location, this._noRetry, this._fullTestName, this._seenNames, @@ -189,6 +201,7 @@ class Declarer { Object? skip, Map? onPlatform, Object? tags, + TestLocation? location, int? retry, bool solo = false}) { _checkNotBuilt('test'); @@ -231,7 +244,10 @@ class Declarer { // Make the declarer visible to running tests so that they'll throw // useful errors when calling `test()` and `group()` within a test. zoneValues: {#test.declarer: this}); - }, trace: _collectTraces ? Trace.current(2) : null, guarded: false)); + }, + trace: _collectTraces ? Trace.current(2) : null, + location: location, + guarded: false)); if (solo) { _soloEntries.add(_entries.last); @@ -245,6 +261,7 @@ class Declarer { Object? skip, Map? onPlatform, Object? tags, + TestLocation? location, int? retry, bool solo = false}) { _checkNotBuilt('group'); @@ -272,6 +289,7 @@ class Declarer { _platformVariables, _collectTraces, trace, + location, _noRetry, _fullTestName, _seenNames, @@ -307,16 +325,18 @@ class Declarer { } /// Registers a function to be run once before all tests. - void setUpAll(dynamic Function() callback) { + void setUpAll(dynamic Function() callback, {TestLocation? location}) { _checkNotBuilt('setUpAll'); if (_collectTraces) _setUpAllTrace ??= Trace.current(2); + _setUpAllLocation ??= location; _setUpAlls.add(callback); } /// Registers a function to be run once after all tests. - void tearDownAll(dynamic Function() callback) { + void tearDownAll(dynamic Function() callback, {TestLocation? location}) { _checkNotBuilt('tearDownAll'); if (_collectTraces) _tearDownAllTrace ??= Trace.current(2); + _tearDownAllLocation ??= location; _tearDownAlls.add(callback); } @@ -347,6 +367,7 @@ class Declarer { return Group(_name ?? '', entries, metadata: _metadata, trace: _trace, + location: _location, setUpAll: _setUpAll, tearDownAll: _tearDownAll); } @@ -393,7 +414,11 @@ class Declarer { // Make the declarer visible to running scaffolds so they can add to // the declarer's `tearDownAll()` list. zoneValues: {#test.declarer: this}); - }, trace: _setUpAllTrace, guarded: false, isScaffoldAll: true); + }, + trace: _setUpAllTrace, + location: _setUpAllLocation, + guarded: false, + isScaffoldAll: true); } /// Returns a [Test] that runs the callbacks in [_tearDownAll], or `null`. @@ -408,7 +433,11 @@ class Declarer { // Make the declarer visible to running scaffolds so they can add to // the declarer's `tearDownAll()` list. zoneValues: {#test.declarer: this}); - }, trace: _tearDownAllTrace, guarded: false, isScaffoldAll: true); + }, + trace: _tearDownAllTrace, + location: _tearDownAllLocation, + guarded: false, + isScaffoldAll: true); } void _addEntry(GroupEntry entry) { diff --git a/pkgs/test_api/lib/src/backend/group.dart b/pkgs/test_api/lib/src/backend/group.dart index 9a2887523..1e9142e17 100644 --- a/pkgs/test_api/lib/src/backend/group.dart +++ b/pkgs/test_api/lib/src/backend/group.dart @@ -8,6 +8,7 @@ import 'group_entry.dart'; import 'metadata.dart'; import 'suite_platform.dart'; import 'test.dart'; +import 'test_location.dart'; /// A group contains one or more tests and subgroups. /// @@ -22,6 +23,9 @@ class Group implements GroupEntry { @override final Trace? trace; + @override + final TestLocation? location; + /// The children of this group. final List entries; @@ -50,7 +54,11 @@ class Group implements GroupEntry { int? _testCount; Group(this.name, Iterable entries, - {Metadata? metadata, this.trace, this.setUpAll, this.tearDownAll}) + {Metadata? metadata, + this.trace, + this.location, + this.setUpAll, + this.tearDownAll}) : entries = List.unmodifiable(entries), metadata = metadata ?? Metadata(); @@ -63,6 +71,7 @@ class Group implements GroupEntry { return Group(name, filtered, metadata: newMetadata, trace: trace, + location: location, setUpAll: setUpAll, tearDownAll: tearDownAll); } @@ -74,6 +83,7 @@ class Group implements GroupEntry { return Group(name, filtered, metadata: metadata, trace: trace, + location: location, setUpAll: setUpAll, tearDownAll: tearDownAll); } diff --git a/pkgs/test_api/lib/src/backend/group_entry.dart b/pkgs/test_api/lib/src/backend/group_entry.dart index a6f30bd9f..85799125b 100644 --- a/pkgs/test_api/lib/src/backend/group_entry.dart +++ b/pkgs/test_api/lib/src/backend/group_entry.dart @@ -2,11 +2,15 @@ // 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. +/// @docImport 'group.dart'; +library; + import 'package:stack_trace/stack_trace.dart'; import 'metadata.dart'; import 'suite_platform.dart'; import 'test.dart'; +import 'test_location.dart'; /// A [Test] or [Group]. abstract class GroupEntry { @@ -24,6 +28,13 @@ abstract class GroupEntry { /// entry, or `null` if the entry was defined in a different way. Trace? get trace; + /// An optional location provided to `test()` or `group()` to support test + /// frameworks like pkg:test_reflective_loader where the test/group location + /// might not be in [trace] at the time `test()` or `group()` are called. + /// + /// If `null`, the location of a test will try to be inferred from [trace]. + TestLocation? get location; + /// Returns a copy of [this] with all platform-specific metadata resolved. /// /// Removes any tests and groups with [Metadata.testOn] selectors that don't diff --git a/pkgs/test_api/lib/src/backend/invoker.dart b/pkgs/test_api/lib/src/backend/invoker.dart index 58f1001d9..0daff47a3 100644 --- a/pkgs/test_api/lib/src/backend/invoker.dart +++ b/pkgs/test_api/lib/src/backend/invoker.dart @@ -18,6 +18,7 @@ import 'suite.dart'; import 'suite_platform.dart'; import 'test.dart'; import 'test_failure.dart'; +import 'test_location.dart'; import 'util/pretty_print.dart'; /// A test in this isolate. @@ -31,6 +32,9 @@ class LocalTest extends Test { @override final Trace? trace; + @override + final TestLocation? location; + /// Whether this is a test defined using `setUpAll()` or `tearDownAll()`. final bool isScaffoldAll; @@ -47,11 +51,14 @@ class LocalTest extends Test { /// the caller's responsibility to invoke [LiveTest.run] in the context of a /// call to [Invoker.guard]. LocalTest(this.name, this.metadata, this._body, - {this.trace, bool guarded = true, this.isScaffoldAll = false}) + {this.trace, + this.location, + bool guarded = true, + this.isScaffoldAll = false}) : _guarded = guarded; - LocalTest._(this.name, this.metadata, this._body, this.trace, this._guarded, - this.isScaffoldAll); + LocalTest._(this.name, this.metadata, this._body, this.trace, this.location, + this._guarded, this.isScaffoldAll); /// Loads a single runnable instance of this test. @override @@ -64,7 +71,7 @@ class LocalTest extends Test { Test? forPlatform(SuitePlatform platform) { if (!metadata.testOn.evaluate(platform)) return null; return LocalTest._(name, metadata.forPlatform(platform), _body, trace, - _guarded, isScaffoldAll); + location, _guarded, isScaffoldAll); } } diff --git a/pkgs/test_api/lib/src/backend/remote_listener.dart b/pkgs/test_api/lib/src/backend/remote_listener.dart index 66a081428..757720904 100644 --- a/pkgs/test_api/lib/src/backend/remote_listener.dart +++ b/pkgs/test_api/lib/src/backend/remote_listener.dart @@ -196,6 +196,7 @@ final class RemoteListener { ?.formatStackTrace(group.trace!) .toString() ?? group.trace?.toString(), + 'location': group.location?.serialize(), 'setUpAll': _serializeTest(channel, group.setUpAll, parents), 'tearDownAll': _serializeTest(channel, group.tearDownAll, parents), 'entries': group.entries.map((entry) { @@ -231,6 +232,7 @@ final class RemoteListener { ?.formatStackTrace(test.trace!) .toString() ?? test.trace?.toString(), + 'location': test.location?.serialize(), 'channel': testChannel.id }; } diff --git a/pkgs/test_api/lib/src/backend/test_location.dart b/pkgs/test_api/lib/src/backend/test_location.dart new file mode 100644 index 000000000..5d9b47637 --- /dev/null +++ b/pkgs/test_api/lib/src/backend/test_location.dart @@ -0,0 +1,27 @@ +// Copyright (c) 2025, 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. + +/// The location of a test or group. +class TestLocation { + final Uri uri; + final int line; + final int column; + + TestLocation(String uri, this.line, this.column) : uri = Uri.parse(uri); + + /// Serializes [this] into a JSON-safe object that can be deserialized using + /// [TestLocation.deserialize]. + Map serialize() { + return { + 'url': uri.toString(), + 'line': line, + 'column': column, + }; + } + + /// Deserializes the result of [TestLocation.serialize] into a new [TestLocation]. + TestLocation.deserialize(Map serialized) + : this(serialized['url'] as String, serialized['line'] as int, + serialized['column'] as int); +} diff --git a/pkgs/test_api/lib/src/scaffolding/test_structure.dart b/pkgs/test_api/lib/src/scaffolding/test_structure.dart index 642e0138a..25b2a852b 100644 --- a/pkgs/test_api/lib/src/scaffolding/test_structure.dart +++ b/pkgs/test_api/lib/src/scaffolding/test_structure.dart @@ -9,6 +9,7 @@ import 'package:meta/meta.dart'; import '../backend/configuration/timeout.dart'; import '../backend/declarer.dart'; import '../backend/invoker.dart'; +import '../backend/test_location.dart'; // test_core does not support running tests directly, so the Declarer should // always be on the Zone. @@ -77,6 +78,7 @@ void test(Object? description, dynamic Function() body, Object? tags, Map? onPlatform, int? retry, + TestLocation? location, // TODO(https://github.com/dart-lang/test/issues/2205): Remove deprecated. @Deprecated('Debug only') @doNotSubmit bool solo = false}) { _declarer.test(description.toString(), body, @@ -86,6 +88,7 @@ void test(Object? description, dynamic Function() body, onPlatform: onPlatform, tags: tags, retry: retry, + location: location, solo: solo); // Force dart2js not to inline this function. We need it to be separate from @@ -156,6 +159,7 @@ void group(Object? description, dynamic Function() body, Object? tags, Map? onPlatform, int? retry, + TestLocation? location, // TODO(https://github.com/dart-lang/test/issues/2205): Remove deprecated. @Deprecated('Debug only') @doNotSubmit bool solo = false}) { _declarer.group(description.toString(), body, @@ -165,6 +169,7 @@ void group(Object? description, dynamic Function() body, tags: tags, onPlatform: onPlatform, retry: retry, + location: location, solo: solo); // Force dart2js not to inline this function. We need it to be separate from @@ -234,7 +239,8 @@ void addTearDown(dynamic Function() callback) { /// dependencies between tests that should be isolated. In general, you should /// prefer [setUp], and only use [setUpAll] if the callback is prohibitively /// slow. -void setUpAll(dynamic Function() callback) => _declarer.setUpAll(callback); +void setUpAll(dynamic Function() callback, {TestLocation? location}) => + _declarer.setUpAll(callback, location: location); /// Registers a function to be run once after all tests. /// @@ -247,5 +253,5 @@ void setUpAll(dynamic Function() callback) => _declarer.setUpAll(callback); /// dependencies between tests that should be isolated. In general, you should /// prefer [tearDown], and only use [tearDownAll] if the callback is /// prohibitively slow. -void tearDownAll(dynamic Function() callback) => - _declarer.tearDownAll(callback); +void tearDownAll(dynamic Function() callback, {TestLocation? location}) => + _declarer.tearDownAll(callback, location: location); diff --git a/pkgs/test_core/lib/src/runner.dart b/pkgs/test_core/lib/src/runner.dart index ee648d46e..eb8ab353b 100644 --- a/pkgs/test_core/lib/src/runner.dart +++ b/pkgs/test_core/lib/src/runner.dart @@ -309,11 +309,13 @@ class Runner { var line = selection.line; var col = selection.col; if (line == null && col == null) return true; + var trace = test.trace; - if (trace == null) { + var location = test.location; + if (trace == null && location == null) { throw StateError( 'Cannot filter by line/column for this test suite, no stack' - 'trace available.'); + 'trace or location available.'); } var path = suite.path; if (path == null) { @@ -324,6 +326,17 @@ class Runner { // The absolute path as it will appear in stack traces. var absoluteSuitePath = File(path).absolute.uri.toFilePath(); + // First check if we're a match for the overridden location. + if (location != null) { + if ((line == null || location.line == line) && + (col == null || location.column == col) && + location.uri.isScheme('file') && + location.uri.toFilePath() == absoluteSuitePath) { + return true; + } + } + + // Next, check if any frames in the stack trace match. bool matchLineAndCol(Frame frame) { switch (frame.uri.scheme) { case 'file': @@ -352,7 +365,7 @@ class Runner { return true; } - return trace.frames.any(matchLineAndCol); + return trace?.frames.any(matchLineAndCol) ?? false; }); })); }); diff --git a/pkgs/test_core/lib/src/runner/plugin/platform_helpers.dart b/pkgs/test_core/lib/src/runner/plugin/platform_helpers.dart index 450dedbc7..241a781bc 100644 --- a/pkgs/test_core/lib/src/runner/plugin/platform_helpers.dart +++ b/pkgs/test_core/lib/src/runner/plugin/platform_helpers.dart @@ -8,7 +8,7 @@ import 'dart:io'; import 'package:stack_trace/stack_trace.dart'; import 'package:stream_channel/stream_channel.dart'; import 'package:test_api/backend.dart' - show Metadata, RemoteException, SuitePlatform; + show Metadata, RemoteException, SuitePlatform, TestLocation; import 'package:test_api/src/backend/group.dart'; // ignore: implementation_imports import 'package:test_api/src/backend/test.dart'; // ignore: implementation_imports @@ -132,6 +132,12 @@ class _Deserializer { /// Deserializes [group] into a concrete [Group]. Group deserializeGroup(Map group) { var metadata = Metadata.deserialize(group['metadata'] as Map); + var trace = + group['trace'] == null ? null : Trace.parse(group['trace'] as String); + var location = switch (group['location']) { + Map map => TestLocation.deserialize(map), + _ => null, + }; return Group( group['name'] as String, (group['entries'] as List).map((entry) { @@ -140,9 +146,8 @@ class _Deserializer { return _deserializeTest(map)!; }), metadata: metadata, - trace: group['trace'] == null - ? null - : Trace.parse(group['trace'] as String), + trace: trace, + location: location, setUpAll: _deserializeTest(group['setUpAll'] as Map?), tearDownAll: _deserializeTest(group['tearDownAll'] as Map?)); } @@ -156,7 +161,12 @@ class _Deserializer { var metadata = Metadata.deserialize(test['metadata'] as Map); var trace = test['trace'] == null ? null : Trace.parse(test['trace'] as String); + var location = switch (test['location']) { + Map map => TestLocation.deserialize(map), + _ => null, + }; var testChannel = _channel.virtualChannel((test['channel'] as num).toInt()); - return RunnerTest(test['name'] as String, metadata, trace, testChannel); + return RunnerTest( + test['name'] as String, metadata, trace, location, testChannel); } } diff --git a/pkgs/test_core/lib/src/runner/reporter/json.dart b/pkgs/test_core/lib/src/runner/reporter/json.dart index b56f030df..e8bca9ff4 100644 --- a/pkgs/test_core/lib/src/runner/reporter/json.dart +++ b/pkgs/test_core/lib/src/runner/reporter/json.dart @@ -16,6 +16,7 @@ import 'package:test_api/src/backend/live_test.dart'; // ignore: implementation_ import 'package:test_api/src/backend/metadata.dart'; // ignore: implementation_imports import 'package:test_api/src/backend/state.dart'; // ignore: implementation_imports import 'package:test_api/src/backend/suite.dart'; // ignore: implementation_imports +import 'package:test_api/src/backend/test_location.dart'; // ignore: implementation_imports import '../../platform.dart'; import '../engine.dart'; @@ -139,7 +140,11 @@ class JsonReporter implements Reporter { 'suiteID': suiteID, 'groupIDs': groupIDs, 'metadata': _serializeMetadata(suiteConfig, liveTest.test.metadata), - ..._frameInfo(suiteConfig, liveTest.test.trace, liveTest.suite.platform, + ..._locationInfo( + suiteConfig, + liveTest.test.trace, + liveTest.test.location, + liveTest.suite.platform, liveTest.suite.path!), } }); @@ -224,7 +229,8 @@ class JsonReporter implements Reporter { 'name': group.name, 'metadata': _serializeMetadata(suiteConfig, group.metadata), 'testCount': group.testCount, - ..._frameInfo(suiteConfig, group.trace, suite.platform, suite.path!) + ..._locationInfo(suiteConfig, group.trace, group.location, + suite.platform, suite.path!) } }); parentID = id; @@ -297,8 +303,21 @@ class JsonReporter implements Reporter { /// If javascript traces are enabled and the test is on a javascript platform, /// or if the [trace] is null or empty, then the line, column, and url will /// all be `null`. - Map _frameInfo(SuiteConfiguration suiteConfig, Trace? trace, - SuitePlatform platform, String suitePath) { + Map _locationInfo( + SuiteConfiguration suiteConfig, + Trace? trace, + TestLocation? location, + SuitePlatform platform, + String suitePath) { + // If this test has a location override, always use that. + if (location != null) { + return { + 'line': location.line, + 'column': location.column, + 'url': location.uri.toString() + }; + } + var absoluteSuitePath = p.canonicalize(p.absolute(suitePath)); var frame = trace?.frames.first; if (frame == null || (suiteConfig.jsTrace && platform.compiler.isJS)) { diff --git a/pkgs/test_core/lib/src/runner/runner_test.dart b/pkgs/test_core/lib/src/runner/runner_test.dart index 2823983e2..207b65ca5 100644 --- a/pkgs/test_core/lib/src/runner/runner_test.dart +++ b/pkgs/test_core/lib/src/runner/runner_test.dart @@ -7,7 +7,7 @@ import 'dart:async'; import 'package:stack_trace/stack_trace.dart'; import 'package:stream_channel/stream_channel.dart'; import 'package:test_api/backend.dart' - show Metadata, RemoteException, SuitePlatform; + show Metadata, RemoteException, SuitePlatform, TestLocation; import 'package:test_api/src/backend/group.dart'; // ignore: implementation_imports import 'package:test_api/src/backend/live_test.dart'; // ignore: implementation_imports import 'package:test_api/src/backend/live_test_controller.dart'; // ignore: implementation_imports @@ -26,11 +26,14 @@ class RunnerTest extends Test { final Metadata metadata; @override final Trace? trace; + @override + final TestLocation? location; /// The channel used to communicate with the test's `RemoteListener`. final MultiChannel _channel; - RunnerTest(this.name, this.metadata, this.trace, this._channel); + RunnerTest( + this.name, this.metadata, this.trace, this.location, this._channel); @override LiveTest load(Suite suite, {Iterable? groups}) { @@ -103,6 +106,7 @@ class RunnerTest extends Test { @override Test? forPlatform(SuitePlatform platform) { if (!metadata.testOn.evaluate(platform)) return null; - return RunnerTest(name, metadata.forPlatform(platform), trace, _channel); + return RunnerTest( + name, metadata.forPlatform(platform), trace, location, _channel); } } diff --git a/pkgs/test_core/lib/src/scaffolding.dart b/pkgs/test_core/lib/src/scaffolding.dart index 31b3d899a..cb28b6203 100644 --- a/pkgs/test_core/lib/src/scaffolding.dart +++ b/pkgs/test_core/lib/src/scaffolding.dart @@ -140,6 +140,7 @@ void test(Object? description, dynamic Function() body, Object? tags, Map? onPlatform, int? retry, + TestLocation? location, // TODO(https://github.com/dart-lang/test/issues/2205): Remove deprecated. @Deprecated('Debug only') @doNotSubmit bool solo = false}) { _declarer.test(description.toString(), body, @@ -149,6 +150,7 @@ void test(Object? description, dynamic Function() body, onPlatform: onPlatform, tags: tags, retry: retry, + location: location, solo: solo); // Force dart2js not to inline this function. We need it to be separate from @@ -219,6 +221,7 @@ void group(Object? description, dynamic Function() body, Object? tags, Map? onPlatform, int? retry, + TestLocation? location, // TODO(https://github.com/dart-lang/test/issues/2205): Remove deprecated. @Deprecated('Debug only') @doNotSubmit bool solo = false}) { _declarer.group(description.toString(), body, @@ -228,6 +231,7 @@ void group(Object? description, dynamic Function() body, tags: tags, onPlatform: onPlatform, retry: retry, + location: location, solo: solo); // Force dart2js not to inline this function. We need it to be separate from @@ -278,7 +282,8 @@ void tearDown(dynamic Function() callback) => _declarer.tearDown(callback); /// dependencies between tests that should be isolated. In general, you should /// prefer [setUp], and only use [setUpAll] if the callback is prohibitively /// slow. -void setUpAll(dynamic Function() callback) => _declarer.setUpAll(callback); +void setUpAll(dynamic Function() callback, {TestLocation? location}) => + _declarer.setUpAll(callback, location: location); /// Registers a function to be run once after all tests. /// @@ -291,5 +296,5 @@ void setUpAll(dynamic Function() callback) => _declarer.setUpAll(callback); /// dependencies between tests that should be isolated. In general, you should /// prefer [tearDown], and only use [tearDownAll] if the callback is /// prohibitively slow. -void tearDownAll(dynamic Function() callback) => - _declarer.tearDownAll(callback); +void tearDownAll(dynamic Function() callback, {TestLocation? location}) => + _declarer.tearDownAll(callback, location: location); From d6c9f36d95b5b246f7758035a8bc8aa0f8cac60f Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Tue, 15 Apr 2025 14:33:57 +0100 Subject: [PATCH 02/13] Make TestLocation constructor take Uri, support filtering org-dartlang-app --- pkgs/test/test/runner/json_reporter_test.dart | 8 +-- pkgs/test/test/runner/line_and_col_test.dart | 62 ++++++++++++------- .../lib/src/backend/test_location.dart | 4 +- pkgs/test_core/lib/src/runner.dart | 44 ++++++++----- 4 files changed, 71 insertions(+), 47 deletions(-) diff --git a/pkgs/test/test/runner/json_reporter_test.dart b/pkgs/test/test/runner/json_reporter_test.dart index b6a6a64cb..88c4c1413 100644 --- a/pkgs/test/test/runner/json_reporter_test.dart +++ b/pkgs/test/test/runner/json_reporter_test.dart @@ -595,10 +595,10 @@ void customTest(String name, dynamic Function() testFn) => test(name, testFn); test('test 1 inferred', () {}); tearDownAll(() {}); }); - group('group 2 custom', location: TestLocation('file:///foo/group', 123, 234), () { - setUpAll(location: TestLocation('file:///foo/setUpAll', 345, 456), () {}); - test('test 2 custom', location: TestLocation('file:///foo/test', 567, 789), () {}); - tearDownAll(location: TestLocation('file:///foo/tearDownAll', 890, 901), () {}); + group('group 2 custom', location: TestLocation(Uri.parse('file:///foo/group'), 123, 234), () { + setUpAll(location: TestLocation(Uri.parse('file:///foo/setUpAll'), 345, 456), () {}); + test('test 2 custom', location: TestLocation(Uri.parse('file:///foo/test'), 567, 789), () {}); + tearDownAll(location: TestLocation(Uri.parse('file:///foo/tearDownAll'), 890, 901), () {}); }); ''', [ [ diff --git a/pkgs/test/test/runner/line_and_col_test.dart b/pkgs/test/test/runner/line_and_col_test.dart index 501600d7d..90c0f0f3f 100644 --- a/pkgs/test/test/runner/line_and_col_test.dart +++ b/pkgs/test/test/runner/line_and_col_test.dart @@ -57,30 +57,36 @@ void main() { }); test('additionally selects test with matching custom location', () async { - var testFileUri = Uri.file(d.file('test.dart').io.path); - var notTestFileUri = Uri.file(d.file('not_test.dart').io.path); - await d.file('test.dart', ''' + await d.dir('test').create(); + var testFileUri = Uri.file(d.file('test/aaa_test.dart').io.path); + var notTestFileUri = Uri.file(d.file('test/bbb.dart').io.path); + var testFilePackageRelativeUri = + Uri.parse('org-dartlang-app:///test/aaa_test.dart'); + await d.file('test/aaa_test.dart', ''' import 'package:test/test.dart'; void main() { - test("a", () {}); // Line 4 from stack trace + test("a", () {}); // Line 4 from stack trace (match) + + // Custom line 4 (match) + test("b", location: TestLocation(Uri.parse('$testFileUri'), 4, 0), () {}); - // Custom line 4 match - test("b", location: TestLocation('$testFileUri', 4, 0), () {}); + // Custom line 4 match but using org-dartlang-app (match) + test("c", location: TestLocation(Uri.parse('$testFilePackageRelativeUri'), 4, 0), () {}); - // Custom different line, same file - test("c", location: TestLocation('$testFileUri', 5, 0), () => throw TestFailure("oh no")); + // Custom different line, same file (no match) + test("d", location: TestLocation(Uri.parse('$testFileUri'), 5, 0), () => throw TestFailure("oh no")); // Custom line 4 match but different file (no match) - test("d", location: TestLocation('$notTestFileUri', 4, 0), () => throw TestFailure("oh no")); + test("e", location: TestLocation(Uri.parse('$notTestFileUri'), 4, 0), () => throw TestFailure("oh no")); } ''').create(); - var test = await runTest(['test.dart?line=4']); + var test = await runTest(['test/aaa_test.dart?line=4']); expect( test.stdout, - emitsThrough(contains('+2: All tests passed!')), + emitsThrough(contains('+3: All tests passed!')), ); await test.shouldExit(0); @@ -119,38 +125,46 @@ void main() { // // So maybe we need to look up the tree to match location (in // Runner._loadSuites() filter)? - var testFileUri = Uri.file(d.file('test.dart').io.path); - var notTestFileUri = Uri.file(d.file('not_test.dart').io.path); - await d.file('test.dart', ''' + await d.dir('test').create(); + var testFileUri = Uri.file(d.file('test/aaa_test.dart').io.path); + var notTestFileUri = Uri.file(d.file('test/bbb.dart').io.path); + var testFilePackageRelativeUri = + Uri.parse('org-dartlang-app:///test/aaa_test.dart'); + await d.file('test/aaa_test.dart', ''' import 'package:test/test.dart'; void main() { - group("a", () { // Line 4 from stack trace + group("a", () { // Line 4 from stack trace (match) test("a", () {}); }); - // Custom line 4 match - group("b", location: TestLocation('$testFileUri', 4, 0), () { + // Custom line 4 (match) + group("b", location: TestLocation(Uri.parse('$testFileUri'), 4, 0), () { test("b", () {}); }); - // Custom different line, same file - group("c", location: TestLocation('$testFileUri', 5, 0), () { - test("c", () => throw TestFailure("oh no")); + // Custom line 4 match but using org-dartlang-app (match) + group("c", location: TestLocation(Uri.parse('$testFilePackageRelativeUri'), 4, 0), () { + test("c", () {}); }); - // Custom line 4 match but different file (no match) - group("d", location: TestLocation('$notTestFileUri', 4, 0), () { + // Custom different line, same file (no match) + group("d", location: TestLocation(Uri.parse('$testFileUri'), 5, 0), () { test("d", () => throw TestFailure("oh no")); }); + + // Custom line 4 match but different file (no match) + group("e", location: TestLocation(Uri.parse('$notTestFileUri'), 4, 0), () { + test("e", () => throw TestFailure("oh no")); + }); } ''').create(); - var test = await runTest(['test.dart?line=4']); + var test = await runTest(['test/aaa_test.dart?line=4']); expect( test.stdout, - emitsThrough(contains('+2: All tests passed!')), + emitsThrough(contains('+3: All tests passed!')), ); await test.shouldExit(0); diff --git a/pkgs/test_api/lib/src/backend/test_location.dart b/pkgs/test_api/lib/src/backend/test_location.dart index 5d9b47637..8d5bea0af 100644 --- a/pkgs/test_api/lib/src/backend/test_location.dart +++ b/pkgs/test_api/lib/src/backend/test_location.dart @@ -8,7 +8,7 @@ class TestLocation { final int line; final int column; - TestLocation(String uri, this.line, this.column) : uri = Uri.parse(uri); + TestLocation(this.uri, this.line, this.column); /// Serializes [this] into a JSON-safe object that can be deserialized using /// [TestLocation.deserialize]. @@ -22,6 +22,6 @@ class TestLocation { /// Deserializes the result of [TestLocation.serialize] into a new [TestLocation]. TestLocation.deserialize(Map serialized) - : this(serialized['url'] as String, serialized['line'] as int, + : this(Uri.parse(serialized['url'] as String), serialized['line'] as int, serialized['column'] as int); } diff --git a/pkgs/test_core/lib/src/runner.dart b/pkgs/test_core/lib/src/runner.dart index eb8ab353b..bc46043b0 100644 --- a/pkgs/test_core/lib/src/runner.dart +++ b/pkgs/test_core/lib/src/runner.dart @@ -324,23 +324,14 @@ class Runner { 'path available.'); } // The absolute path as it will appear in stack traces. - var absoluteSuitePath = File(path).absolute.uri.toFilePath(); + var suiteUri = File(path).absolute.uri; + var absoluteSuitePath = suiteUri.toFilePath(); - // First check if we're a match for the overridden location. - if (location != null) { - if ((line == null || location.line == line) && - (col == null || location.column == col) && - location.uri.isScheme('file') && - location.uri.toFilePath() == absoluteSuitePath) { - return true; - } - } - - // Next, check if any frames in the stack trace match. - bool matchLineAndCol(Frame frame) { - switch (frame.uri.scheme) { + /// Helper to check if [uri] matches the suite path. + bool matchesUri(Uri uri) { + switch (uri.scheme) { case 'file': - if (frame.uri.toFilePath() != absoluteSuitePath) { + if (uri.toFilePath() != absoluteSuitePath) { return false; } case 'package': @@ -351,11 +342,29 @@ class Runner { // Now we can assume that the kernel is compiled using // --filesystem-scheme. // In this case, because we don't know the --filesystem-root, as - // long as the file path matches we assume it is the same file. - if (!absoluteSuitePath.endsWith(frame.uri.path)) { + // long as the path matches we assume it is the same file. + if (!suiteUri.path.endsWith(uri.path)) { return false; } } + + return true; + } + + // First check if we're a match for the overridden location. + if (location != null) { + if ((line == null || location.line == line) && + (col == null || location.column == col) && + matchesUri(location.uri)) { + return true; + } + } + + /// Helper to check if [frame] matches the suite path, line and col. + bool matchLineAndCol(Frame frame) { + if (!matchesUri(frame.uri)) { + return false; + } if (line != null && frame.line != line) { return false; } @@ -365,6 +374,7 @@ class Runner { return true; } + // Check if any frames in the stack trace match. return trace?.frames.any(matchLineAndCol) ?? false; }); })); From efb19e2a8c9d80ff4feb39148bbac14c24031077 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 23 Apr 2025 11:47:40 +0100 Subject: [PATCH 03/13] Use location.serialize in the JSON reporter --- pkgs/test_api/lib/src/backend/test_location.dart | 3 +++ pkgs/test_core/lib/src/runner/reporter/json.dart | 6 +----- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/pkgs/test_api/lib/src/backend/test_location.dart b/pkgs/test_api/lib/src/backend/test_location.dart index 8d5bea0af..4f16079de 100644 --- a/pkgs/test_api/lib/src/backend/test_location.dart +++ b/pkgs/test_api/lib/src/backend/test_location.dart @@ -12,6 +12,9 @@ class TestLocation { /// Serializes [this] into a JSON-safe object that can be deserialized using /// [TestLocation.deserialize]. + /// + /// This method is also used to provide the location in the JSON reporter when + /// a custom location is provided for the test. Map serialize() { return { 'url': uri.toString(), diff --git a/pkgs/test_core/lib/src/runner/reporter/json.dart b/pkgs/test_core/lib/src/runner/reporter/json.dart index e8bca9ff4..9f582eeab 100644 --- a/pkgs/test_core/lib/src/runner/reporter/json.dart +++ b/pkgs/test_core/lib/src/runner/reporter/json.dart @@ -311,11 +311,7 @@ class JsonReporter implements Reporter { String suitePath) { // If this test has a location override, always use that. if (location != null) { - return { - 'line': location.line, - 'column': location.column, - 'url': location.uri.toString() - }; + return location.serialize(); } var absoluteSuitePath = p.canonicalize(p.absolute(suitePath)); From 2044d02946f10408d5842cc49c794f860d906a6c Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 23 Apr 2025 12:57:33 +0100 Subject: [PATCH 04/13] Add GroupEntry.parent and wire up in declarer + deserialization --- pkgs/test/test/runner/line_and_col_test.dart | 7 ------ pkgs/test_api/lib/src/backend/declarer.dart | 12 +++++++++- pkgs/test_api/lib/src/backend/group.dart | 3 +++ .../test_api/lib/src/backend/group_entry.dart | 10 +++++--- pkgs/test_api/lib/src/backend/test.dart | 3 +++ pkgs/test_core/lib/src/runner.dart | 21 +++++++++------- pkgs/test_core/lib/src/runner/engine.dart | 3 ++- .../src/runner/plugin/platform_helpers.dart | 24 +++++++++++++------ .../test_core/lib/src/runner/runner_test.dart | 6 ++++- 9 files changed, 61 insertions(+), 28 deletions(-) diff --git a/pkgs/test/test/runner/line_and_col_test.dart b/pkgs/test/test/runner/line_and_col_test.dart index 90c0f0f3f..3241c4f3f 100644 --- a/pkgs/test/test/runner/line_and_col_test.dart +++ b/pkgs/test/test/runner/line_and_col_test.dart @@ -118,13 +118,6 @@ void main() { test('additionally selects groups with a matching custom location', () async { - // TODO(dantup): This fails because we don't ever look at a groups - // location.. instead, we only look at tests. The reason we can - // normally run groups by line/col is because they just happen to be - // in the stack trace for the test() call too. - // - // So maybe we need to look up the tree to match location (in - // Runner._loadSuites() filter)? await d.dir('test').create(); var testFileUri = Uri.file(d.file('test/aaa_test.dart').io.path); var notTestFileUri = Uri.file(d.file('test/bbb.dart').io.path); diff --git a/pkgs/test_api/lib/src/backend/declarer.dart b/pkgs/test_api/lib/src/backend/declarer.dart index 4e9818d1e..14ab40498 100644 --- a/pkgs/test_api/lib/src/backend/declarer.dart +++ b/pkgs/test_api/lib/src/backend/declarer.dart @@ -364,12 +364,22 @@ class Declarer { return entry; }).toList(); - return Group(_name ?? '', entries, + var result = Group(_name ?? '', entries, metadata: _metadata, trace: _trace, location: _location, setUpAll: _setUpAll, tearDownAll: _tearDownAll); + + // Now we have created the group, we can set it as the parent for all + // child entries. + for (var entry in result.entries) { + entry.parent = result; + } + result.setUpAll?.parent = result; + result.tearDownAll?.parent = result; + + return result; } /// Throws a [StateError] if [build] has been called. diff --git a/pkgs/test_api/lib/src/backend/group.dart b/pkgs/test_api/lib/src/backend/group.dart index 1e9142e17..612d96efd 100644 --- a/pkgs/test_api/lib/src/backend/group.dart +++ b/pkgs/test_api/lib/src/backend/group.dart @@ -17,6 +17,9 @@ class Group implements GroupEntry { @override final String name; + @override + Group? parent; + @override final Metadata metadata; diff --git a/pkgs/test_api/lib/src/backend/group_entry.dart b/pkgs/test_api/lib/src/backend/group_entry.dart index 85799125b..5246efbcc 100644 --- a/pkgs/test_api/lib/src/backend/group_entry.dart +++ b/pkgs/test_api/lib/src/backend/group_entry.dart @@ -2,11 +2,9 @@ // 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. -/// @docImport 'group.dart'; -library; - import 'package:stack_trace/stack_trace.dart'; +import 'group.dart'; import 'metadata.dart'; import 'suite_platform.dart'; import 'test.dart'; @@ -20,6 +18,12 @@ abstract class GroupEntry { /// This will be empty for the root group. String get name; + /// The parent of this entry. + /// + /// This field is set during during building in the Declarer and also during + /// deserialization of the parent. + Group? parent; + /// The metadata for the entry, including the metadata from any containing /// [Group]s. Metadata get metadata; diff --git a/pkgs/test_api/lib/src/backend/test.dart b/pkgs/test_api/lib/src/backend/test.dart index 5b5e73a67..ae848dd48 100644 --- a/pkgs/test_api/lib/src/backend/test.dart +++ b/pkgs/test_api/lib/src/backend/test.dart @@ -20,6 +20,9 @@ abstract class Test implements GroupEntry { @override String get name; + @override + Group? parent; + @override Metadata get metadata; diff --git a/pkgs/test_core/lib/src/runner.dart b/pkgs/test_core/lib/src/runner.dart index bc46043b0..354fc5e9f 100644 --- a/pkgs/test_core/lib/src/runner.dart +++ b/pkgs/test_core/lib/src/runner.dart @@ -311,8 +311,7 @@ class Runner { if (line == null && col == null) return true; var trace = test.trace; - var location = test.location; - if (trace == null && location == null) { + if (trace == null && test.location == null) { throw StateError( 'Cannot filter by line/column for this test suite, no stack' 'trace or location available.'); @@ -351,13 +350,19 @@ class Runner { return true; } - // First check if we're a match for the overridden location. - if (location != null) { - if ((line == null || location.line == line) && - (col == null || location.column == col) && - matchesUri(location.uri)) { - return true; + // First check if we're a match for the overridden location for this + // item or any parents. + var current = test as GroupEntry?; + while (current != null) { + var location = current.location; + if (location != null) { + if ((line == null || location.line == line) && + (col == null || location.column == col) && + matchesUri(location.uri)) { + return true; + } } + current = current.parent; } /// Helper to check if [frame] matches the suite path, line and col. diff --git a/pkgs/test_core/lib/src/runner/engine.dart b/pkgs/test_core/lib/src/runner/engine.dart index 6a6362761..4404d1c5c 100644 --- a/pkgs/test_core/lib/src/runner/engine.dart +++ b/pkgs/test_core/lib/src/runner/engine.dart @@ -404,7 +404,8 @@ class Engine { Future _runSkippedTest(LiveSuiteController suiteController, Test test, List parents) async { await _onUnpaused; - var skipped = LocalTest(test.name, test.metadata, () {}, trace: test.trace); + var skipped = LocalTest(test.name, test.metadata, () {}, + trace: test.trace, location: test.location); late LiveTestController controller; controller = diff --git a/pkgs/test_core/lib/src/runner/plugin/platform_helpers.dart b/pkgs/test_core/lib/src/runner/plugin/platform_helpers.dart index 241a781bc..ca1ffe73e 100644 --- a/pkgs/test_core/lib/src/runner/plugin/platform_helpers.dart +++ b/pkgs/test_core/lib/src/runner/plugin/platform_helpers.dart @@ -138,18 +138,28 @@ class _Deserializer { Map map => TestLocation.deserialize(map), _ => null, }; - return Group( - group['name'] as String, - (group['entries'] as List).map((entry) { - var map = entry as Map; - if (map['type'] == 'group') return deserializeGroup(map); - return _deserializeTest(map)!; - }), + var entries = (group['entries'] as List).map((entry) { + var map = entry as Map; + if (map['type'] == 'group') return deserializeGroup(map); + return _deserializeTest(map)!; + }).toList(); + + var result = Group(group['name'] as String, entries, metadata: metadata, trace: trace, location: location, setUpAll: _deserializeTest(group['setUpAll'] as Map?), tearDownAll: _deserializeTest(group['tearDownAll'] as Map?)); + + // Now we have created the group, we can set it as the parent for all + // child entries. + for (var entry in result.entries) { + entry.parent = result; + } + result.setUpAll?.parent = result; + result.tearDownAll?.parent = result; + + return result; } /// Deserializes [test] into a concrete [Test] class. diff --git a/pkgs/test_core/lib/src/runner/runner_test.dart b/pkgs/test_core/lib/src/runner/runner_test.dart index 207b65ca5..1162df46c 100644 --- a/pkgs/test_core/lib/src/runner/runner_test.dart +++ b/pkgs/test_core/lib/src/runner/runner_test.dart @@ -106,7 +106,11 @@ class RunnerTest extends Test { @override Test? forPlatform(SuitePlatform platform) { if (!metadata.testOn.evaluate(platform)) return null; - return RunnerTest( + var result = RunnerTest( name, metadata.forPlatform(platform), trace, location, _channel); + // Parents are copied after construction, since usually we build + // bottom-up. + result.parent = parent; + return result; } } From d680e7cb496ca136296ec57d993068c6c1285406 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 23 Apr 2025 16:45:18 +0100 Subject: [PATCH 05/13] Move re-parenting to Group() constructor + update changelog --- .vscode/settings.json | 3 +++ pkgs/test/CHANGELOG.md | 4 ++++ pkgs/test_api/CHANGELOG.md | 6 ++++++ pkgs/test_api/lib/src/backend/declarer.dart | 12 +----------- pkgs/test_api/lib/src/backend/group.dart | 11 ++++++++++- pkgs/test_api/pubspec.yaml | 2 +- pkgs/test_core/CHANGELOG.md | 3 +++ pkgs/test_core/lib/src/runner.dart | 3 +-- .../lib/src/runner/plugin/platform_helpers.dart | 12 +----------- 9 files changed, 30 insertions(+), 26 deletions(-) create mode 100644 .vscode/settings.json diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 000000000..407e6f8b2 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "dart.sdkPath": "D:\\Tools\\Dart\\Latest (Bleeding Edge)" +} diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md index 8ca0f96ab..fdf8dc4f9 100644 --- a/pkgs/test/CHANGELOG.md +++ b/pkgs/test/CHANGELOG.md @@ -1,5 +1,9 @@ ## 1.25.16-wip +* `test()` and `group()` functions now take an optional `TestLocation` that will + be used as the location of the test in JSON reporters instead of being parsed + from the call stack. + ## 1.25.15 * Allow the latest version of `package:shelf_web_socket`. diff --git a/pkgs/test_api/CHANGELOG.md b/pkgs/test_api/CHANGELOG.md index 98d65baea..9f25c2de8 100644 --- a/pkgs/test_api/CHANGELOG.md +++ b/pkgs/test_api/CHANGELOG.md @@ -1,3 +1,9 @@ +## 0.7.5-wip + +* `test()` and `group()` functions now take an optional `TestLocation` that will + be used as the location of the test in JSON reporters instead of being parsed + from the call stack. + ## 0.7.4 * Allow `analyzer: '>=6.0.0 <8.0.0'` diff --git a/pkgs/test_api/lib/src/backend/declarer.dart b/pkgs/test_api/lib/src/backend/declarer.dart index 14ab40498..4e9818d1e 100644 --- a/pkgs/test_api/lib/src/backend/declarer.dart +++ b/pkgs/test_api/lib/src/backend/declarer.dart @@ -364,22 +364,12 @@ class Declarer { return entry; }).toList(); - var result = Group(_name ?? '', entries, + return Group(_name ?? '', entries, metadata: _metadata, trace: _trace, location: _location, setUpAll: _setUpAll, tearDownAll: _tearDownAll); - - // Now we have created the group, we can set it as the parent for all - // child entries. - for (var entry in result.entries) { - entry.parent = result; - } - result.setUpAll?.parent = result; - result.tearDownAll?.parent = result; - - return result; } /// Throws a [StateError] if [build] has been called. diff --git a/pkgs/test_api/lib/src/backend/group.dart b/pkgs/test_api/lib/src/backend/group.dart index 612d96efd..61806bd87 100644 --- a/pkgs/test_api/lib/src/backend/group.dart +++ b/pkgs/test_api/lib/src/backend/group.dart @@ -63,7 +63,16 @@ class Group implements GroupEntry { this.setUpAll, this.tearDownAll}) : entries = List.unmodifiable(entries), - metadata = metadata ?? Metadata(); + metadata = metadata ?? Metadata() { + for (var entry in entries) { + assert(entry.parent == null); + entry.parent = this; + } + assert(setUpAll?.parent == null); + setUpAll?.parent = this; + assert(tearDownAll?.parent == null); + tearDownAll?.parent = this; + } @override Group? forPlatform(SuitePlatform platform) { diff --git a/pkgs/test_api/pubspec.yaml b/pkgs/test_api/pubspec.yaml index 09cdd39a8..2fd43485a 100644 --- a/pkgs/test_api/pubspec.yaml +++ b/pkgs/test_api/pubspec.yaml @@ -1,5 +1,5 @@ name: test_api -version: 0.7.4 +version: 0.7.5-wip description: >- The user facing API for structuring Dart tests and checking expectations. repository: https://github.com/dart-lang/test/tree/master/pkgs/test_api diff --git a/pkgs/test_core/CHANGELOG.md b/pkgs/test_core/CHANGELOG.md index 9935d9dc0..32db1f393 100644 --- a/pkgs/test_core/CHANGELOG.md +++ b/pkgs/test_core/CHANGELOG.md @@ -1,6 +1,9 @@ ## 0.6.9-wip - Add support for native assets for `dart test` in pub workspaces. +* `test()` and `group()` functions now take an optional `TestLocation` that will + be used as the location of the test in JSON reporters instead of being parsed + from the call stack. ## 0.6.8 diff --git a/pkgs/test_core/lib/src/runner.dart b/pkgs/test_core/lib/src/runner.dart index 354fc5e9f..af8e9c96d 100644 --- a/pkgs/test_core/lib/src/runner.dart +++ b/pkgs/test_core/lib/src/runner.dart @@ -354,8 +354,7 @@ class Runner { // item or any parents. var current = test as GroupEntry?; while (current != null) { - var location = current.location; - if (location != null) { + if (current.location case var location?) { if ((line == null || location.line == line) && (col == null || location.column == col) && matchesUri(location.uri)) { diff --git a/pkgs/test_core/lib/src/runner/plugin/platform_helpers.dart b/pkgs/test_core/lib/src/runner/plugin/platform_helpers.dart index ca1ffe73e..e64ed04dd 100644 --- a/pkgs/test_core/lib/src/runner/plugin/platform_helpers.dart +++ b/pkgs/test_core/lib/src/runner/plugin/platform_helpers.dart @@ -144,22 +144,12 @@ class _Deserializer { return _deserializeTest(map)!; }).toList(); - var result = Group(group['name'] as String, entries, + return Group(group['name'] as String, entries, metadata: metadata, trace: trace, location: location, setUpAll: _deserializeTest(group['setUpAll'] as Map?), tearDownAll: _deserializeTest(group['tearDownAll'] as Map?)); - - // Now we have created the group, we can set it as the parent for all - // child entries. - for (var entry in result.entries) { - entry.parent = result; - } - result.setUpAll?.parent = result; - result.tearDownAll?.parent = result; - - return result; } /// Deserializes [test] into a concrete [Test] class. From 35a6ef0945b8d29d32bc60ae2791900dcfa2642f Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 23 Apr 2025 16:52:54 +0100 Subject: [PATCH 06/13] Delete settings.json --- .vscode/settings.json | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 .vscode/settings.json diff --git a/.vscode/settings.json b/.vscode/settings.json deleted file mode 100644 index 407e6f8b2..000000000 --- a/.vscode/settings.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "dart.sdkPath": "D:\\Tools\\Dart\\Latest (Bleeding Edge)" -} From 675418f39735cbc1fdc2e5497f736af373eebb8e Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 23 Apr 2025 16:53:15 +0100 Subject: [PATCH 07/13] Update .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 56c3800bf..66c982fe0 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ # Don’t commit the following directories created by pub. .buildlog .dart_tool/ +.vscode/settings.json .pub/ build/ packages From 916ecbd3f6d92ee54bc6803d828ac5eb7c241507 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 23 Apr 2025 17:19:23 +0100 Subject: [PATCH 08/13] Update CHANGELOG.md --- pkgs/test_core/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/test_core/CHANGELOG.md b/pkgs/test_core/CHANGELOG.md index 32db1f393..c6423f719 100644 --- a/pkgs/test_core/CHANGELOG.md +++ b/pkgs/test_core/CHANGELOG.md @@ -1,6 +1,6 @@ ## 0.6.9-wip -- Add support for native assets for `dart test` in pub workspaces. +* Add support for native assets for `dart test` in pub workspaces. * `test()` and `group()` functions now take an optional `TestLocation` that will be used as the location of the test in JSON reporters instead of being parsed from the call stack. From e584b33aa2f82ae529ed0517ce962d5f934ed9a4 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Thu, 24 Apr 2025 15:37:18 +0100 Subject: [PATCH 09/13] Bump version --- pkgs/test/CHANGELOG.md | 2 +- pkgs/test/pubspec.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md index fdf8dc4f9..cf945f108 100644 --- a/pkgs/test/CHANGELOG.md +++ b/pkgs/test/CHANGELOG.md @@ -1,4 +1,4 @@ -## 1.25.16-wip +## 1.26.0-wip * `test()` and `group()` functions now take an optional `TestLocation` that will be used as the location of the test in JSON reporters instead of being parsed diff --git a/pkgs/test/pubspec.yaml b/pkgs/test/pubspec.yaml index 78dabdeda..58edf7716 100644 --- a/pkgs/test/pubspec.yaml +++ b/pkgs/test/pubspec.yaml @@ -1,5 +1,5 @@ name: test -version: 1.25.16-wip +version: 1.26.0-wip description: >- A full featured library for writing and running Dart tests across platforms. repository: https://github.com/dart-lang/test/tree/master/pkgs/test From 888a9e83c145ce514d20dba084f9575c84b30cfc Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Thu, 24 Apr 2025 15:42:45 +0100 Subject: [PATCH 10/13] Fix versions --- pkgs/test/pubspec.yaml | 2 +- pkgs/test_core/pubspec.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/test/pubspec.yaml b/pkgs/test/pubspec.yaml index 58edf7716..831485c37 100644 --- a/pkgs/test/pubspec.yaml +++ b/pkgs/test/pubspec.yaml @@ -36,7 +36,7 @@ dependencies: stream_channel: ^2.1.0 # Use an exact version until the test_api and test_core package are stable. - test_api: 0.7.4 + test_api: 0.7.5-wip test_core: 0.6.9-wip typed_data: ^1.3.0 diff --git a/pkgs/test_core/pubspec.yaml b/pkgs/test_core/pubspec.yaml index e5655b974..bf8cc83dc 100644 --- a/pkgs/test_core/pubspec.yaml +++ b/pkgs/test_core/pubspec.yaml @@ -28,7 +28,7 @@ dependencies: stack_trace: ^1.10.0 stream_channel: ^2.1.0 # Use an exact version until the test_api package is stable. - test_api: 0.7.4 + test_api: 0.7.5-wip vm_service: ">=6.0.0 <16.0.0" yaml: ^3.0.0 From 990f13faf2e54307f92d9d9aed571ea77c52780a Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Thu, 24 Apr 2025 16:04:22 +0100 Subject: [PATCH 11/13] Clone setup/teardown with forPlatform() --- pkgs/test_api/lib/src/backend/group.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/test_api/lib/src/backend/group.dart b/pkgs/test_api/lib/src/backend/group.dart index 61806bd87..d90b29d8e 100644 --- a/pkgs/test_api/lib/src/backend/group.dart +++ b/pkgs/test_api/lib/src/backend/group.dart @@ -84,8 +84,8 @@ class Group implements GroupEntry { metadata: newMetadata, trace: trace, location: location, - setUpAll: setUpAll, - tearDownAll: tearDownAll); + setUpAll: setUpAll?.forPlatform(platform), + tearDownAll: tearDownAll?.forPlatform(platform)); } @override From 4dcac4b5af80e061fa415d7d7dd64332424e6ed7 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Thu, 24 Apr 2025 16:22:12 +0100 Subject: [PATCH 12/13] Remove unnecessary parent assignment --- pkgs/test_core/lib/src/runner/runner_test.dart | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/pkgs/test_core/lib/src/runner/runner_test.dart b/pkgs/test_core/lib/src/runner/runner_test.dart index 1162df46c..207b65ca5 100644 --- a/pkgs/test_core/lib/src/runner/runner_test.dart +++ b/pkgs/test_core/lib/src/runner/runner_test.dart @@ -106,11 +106,7 @@ class RunnerTest extends Test { @override Test? forPlatform(SuitePlatform platform) { if (!metadata.testOn.evaluate(platform)) return null; - var result = RunnerTest( + return RunnerTest( name, metadata.forPlatform(platform), trace, location, _channel); - // Parents are copied after construction, since usually we build - // bottom-up. - result.parent = parent; - return result; } } From 240e4e633cd896ec7e331c38fdc5164f950538b5 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Fri, 25 Apr 2025 11:41:26 +0100 Subject: [PATCH 13/13] Clone tests in filter() and in tests that assign to new groups --- pkgs/test/test/runner/engine_test.dart | 21 +++++++++++++------ pkgs/test_api/lib/src/backend/invoker.dart | 12 +++++++++++ pkgs/test_api/lib/src/backend/test.dart | 2 +- .../test_core/lib/src/runner/runner_test.dart | 11 ++++++++++ 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/pkgs/test/test/runner/engine_test.dart b/pkgs/test/test/runner/engine_test.dart index 6b7df23e8..6e8bf3deb 100644 --- a/pkgs/test/test/runner/engine_test.dart +++ b/pkgs/test/test/runner/engine_test.dart @@ -7,6 +7,7 @@ import 'dart:math'; import 'package:test/test.dart'; import 'package:test_api/src/backend/group.dart'; +import 'package:test_api/src/backend/group_entry.dart'; import 'package:test_api/src/backend/state.dart'; import 'package:test_core/src/runner/engine.dart'; @@ -27,8 +28,8 @@ void main() { }); var engine = Engine.withSuites([ - runnerSuite(Group.root(tests.take(2))), - runnerSuite(Group.root(tests.skip(2))) + runnerSuite(tests.take(2).asRootGroup()), + runnerSuite(tests.skip(2).asRootGroup()) ]); await engine.run(); @@ -55,7 +56,7 @@ void main() { }), completes); - engine.suiteSink.add(runnerSuite(Group.root(tests))); + engine.suiteSink.add(runnerSuite(tests.asRootGroup())); engine.suiteSink.close(); }); @@ -207,7 +208,7 @@ void main() { test('test', () {}, skip: true); }); - var engine = Engine.withSuites([runnerSuite(Group.root(tests))]); + var engine = Engine.withSuites([runnerSuite(tests.asRootGroup())]); engine.onTestStarted.listen(expectAsync1((liveTest) { expect(liveTest, same(engine.liveTests.single)); @@ -276,7 +277,7 @@ void main() { }, skip: true); }); - var engine = Engine.withSuites([runnerSuite(Group.root(entries))]); + var engine = Engine.withSuites([runnerSuite(entries.asRootGroup())]); engine.onTestStarted.listen(expectAsync1((liveTest) { expect(liveTest, same(engine.liveTests.single)); @@ -341,7 +342,7 @@ void main() { for (var i = 0; i < testCount; i++) loadSuite('group $i', () async { await updateAndCheckConcurrency(isLoadSuite: true); - return runnerSuite(Group.root([tests[i]])); + return runnerSuite([tests[i]].asRootGroup()); }), ], concurrency: concurrency); @@ -355,3 +356,11 @@ void main() { }); }); } + +extension on Iterable { + /// Clones these entries into a new root group, assigning the new parent group + /// as necessary. + Group asRootGroup() { + return Group.root(map((entry) => entry.filter((_) => true)!)); + } +} diff --git a/pkgs/test_api/lib/src/backend/invoker.dart b/pkgs/test_api/lib/src/backend/invoker.dart index 0daff47a3..412e7531e 100644 --- a/pkgs/test_api/lib/src/backend/invoker.dart +++ b/pkgs/test_api/lib/src/backend/invoker.dart @@ -73,6 +73,18 @@ class LocalTest extends Test { return LocalTest._(name, metadata.forPlatform(platform), _body, trace, location, _guarded, isScaffoldAll); } + + @override + Test? filter(bool Function(Test) callback) { + if (callback(this)) { + // filter() always returns new copies because they need to be attached + // to their new parents. + return LocalTest._( + name, metadata, _body, trace, location, _guarded, isScaffoldAll); + } + + return null; + } } /// The class responsible for managing the lifecycle of a single local test. diff --git a/pkgs/test_api/lib/src/backend/test.dart b/pkgs/test_api/lib/src/backend/test.dart index ae848dd48..45ecde53c 100644 --- a/pkgs/test_api/lib/src/backend/test.dart +++ b/pkgs/test_api/lib/src/backend/test.dart @@ -41,5 +41,5 @@ abstract class Test implements GroupEntry { Test? forPlatform(SuitePlatform platform); @override - Test? filter(bool Function(Test) callback) => callback(this) ? this : null; + Test? filter(bool Function(Test) callback); } diff --git a/pkgs/test_core/lib/src/runner/runner_test.dart b/pkgs/test_core/lib/src/runner/runner_test.dart index 207b65ca5..677562477 100644 --- a/pkgs/test_core/lib/src/runner/runner_test.dart +++ b/pkgs/test_core/lib/src/runner/runner_test.dart @@ -109,4 +109,15 @@ class RunnerTest extends Test { return RunnerTest( name, metadata.forPlatform(platform), trace, location, _channel); } + + @override + Test? filter(bool Function(Test) callback) { + if (callback(this)) { + // filter() always returns new copies because they need to be attached + // to their new parents. + return RunnerTest(name, metadata, trace, location, _channel); + } + + return null; + } }