From f54aadb19de9b8ae8f8cbf66eba798262c914a93 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Tue, 13 May 2025 11:33:43 +0100 Subject: [PATCH 1/6] [test_reflective_loader] Pass test locations to `pkg:test` to improve IDE navigation This updates `pkg:test_reflective_loader` to the latest version of `pkg:test` and passes the locations of tests declarations through to `test()` calls. These locations are then used by `pkg:test` in the JSON reporter so that IDEs can navigate to the correct locations of these tests (rather than always to the `defineReflectiveTests()` call, because the real test declarations are not in the call stack at the point that `test()` is called). Fixes https://github.com/Dart-Code/Dart-Code/issues/5480 --- pkgs/test_reflective_loader/CHANGELOG.md | 6 ++ .../lib/test_reflective_loader.dart | 37 +++++++--- pkgs/test_reflective_loader/pubspec.yaml | 5 +- .../test/location_test.dart | 69 +++++++++++++++++++ 4 files changed, 105 insertions(+), 12 deletions(-) create mode 100644 pkgs/test_reflective_loader/test/location_test.dart diff --git a/pkgs/test_reflective_loader/CHANGELOG.md b/pkgs/test_reflective_loader/CHANGELOG.md index 803eb0e0c..acecb514a 100644 --- a/pkgs/test_reflective_loader/CHANGELOG.md +++ b/pkgs/test_reflective_loader/CHANGELOG.md @@ -1,3 +1,9 @@ +## 0.3.0 + +- Update to `package:test` 1.26.0. +- Pass locations of groups/tests to `package:test` to improve locations reported + in the JSON reporter that may be used for navigation in IDEs. + ## 0.2.3 - Require Dart `^3.1.0`. diff --git a/pkgs/test_reflective_loader/lib/test_reflective_loader.dart b/pkgs/test_reflective_loader/lib/test_reflective_loader.dart index cb69bf3ba..f2e4928c8 100644 --- a/pkgs/test_reflective_loader/lib/test_reflective_loader.dart +++ b/pkgs/test_reflective_loader/lib/test_reflective_loader.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'dart:mirrors'; +import 'package:test/scaffolding.dart'; import 'package:test/test.dart' as test_package; /// A marker annotation used to annotate test methods which are expected to fail @@ -87,7 +88,8 @@ void defineReflectiveTests(Type type) { { var isSolo = _hasAnnotationInstance(classMirror, soloTest); var className = MirrorSystem.getName(classMirror.simpleName); - group = _Group(isSolo, _combineNames(_currentSuiteName, className)); + group = _Group(isSolo, _combineNames(_currentSuiteName, className), + classMirror.testLocation); _currentGroups.add(group); } @@ -104,7 +106,7 @@ void defineReflectiveTests(Type type) { // test_ if (memberName.startsWith('test_')) { if (_hasSkippedTestAnnotation(memberMirror)) { - group.addSkippedTest(memberName); + group.addSkippedTest(memberName, memberMirror.testLocation); } else { group.addTest(isSolo, memberName, memberMirror, () { if (_hasFailingTestAnnotation(memberMirror) || @@ -137,7 +139,7 @@ void defineReflectiveTests(Type type) { } // skip_test_ if (memberName.startsWith('skip_test_')) { - group.addSkippedTest(memberName); + group.addSkippedTest(memberName, memberMirror.testLocation); } }); @@ -154,7 +156,9 @@ void _addTestsIfTopLevelSuite() { for (var test in group.tests) { if (allTests || test.isSolo) { test_package.test(test.name, test.function, - timeout: test.timeout, skip: test.isSkipped); + timeout: test.timeout, + skip: test.isSkipped, + location: test.location); } } } @@ -304,15 +308,16 @@ class _AssertFailingTest { class _Group { final bool isSolo; final String name; + final TestLocation? location; final List<_Test> tests = <_Test>[]; - _Group(this.isSolo, this.name); + _Group(this.isSolo, this.name, this.location); bool get hasSoloTest => tests.any((test) => test.isSolo); - void addSkippedTest(String name) { + void addSkippedTest(String name, TestLocation? location) { var fullName = _combineNames(this.name, name); - tests.add(_Test.skipped(isSolo, fullName)); + tests.add(_Test.skipped(isSolo, fullName, location)); } void addTest(bool isSolo, String name, MethodMirror memberMirror, @@ -320,7 +325,8 @@ class _Group { var fullName = _combineNames(this.name, name); var timeout = _getAnnotationInstance(memberMirror, TestTimeout) as TestTimeout?; - tests.add(_Test(isSolo, fullName, function, timeout?._timeout)); + tests.add(_Test(isSolo, fullName, function, timeout?._timeout, + memberMirror.testLocation)); } } @@ -341,14 +347,25 @@ class _Test { final String name; final _TestFunction function; final test_package.Timeout? timeout; + final TestLocation? location; final bool isSkipped; - _Test(this.isSolo, this.name, this.function, this.timeout) + _Test(this.isSolo, this.name, this.function, this.timeout, this.location) : isSkipped = false; - _Test.skipped(this.isSolo, this.name) + _Test.skipped(this.isSolo, this.name, this.location) : isSkipped = true, function = (() {}), timeout = null; } + +extension on DeclarationMirror { + TestLocation? get testLocation { + if (location case var location?) { + return TestLocation(location.sourceUri, location.line, location.column); + } else { + return null; + } + } +} diff --git a/pkgs/test_reflective_loader/pubspec.yaml b/pkgs/test_reflective_loader/pubspec.yaml index f63ab0140..47da4d8b3 100644 --- a/pkgs/test_reflective_loader/pubspec.yaml +++ b/pkgs/test_reflective_loader/pubspec.yaml @@ -1,5 +1,5 @@ name: test_reflective_loader -version: 0.2.3 +version: 0.3.0 description: Support for discovering tests and test suites using reflection. repository: https://github.com/dart-lang/tools/tree/main/pkgs/test_reflective_loader issue_tracker: https://github.com/dart-lang/tools/labels/package%3Atest_reflective_loader @@ -8,7 +8,8 @@ environment: sdk: ^3.1.0 dependencies: - test: ^1.16.0 + test: ^1.26.0 dev_dependencies: dart_flutter_team_lints: ^3.0.0 + path: ^1.8.0 diff --git a/pkgs/test_reflective_loader/test/location_test.dart b/pkgs/test_reflective_loader/test/location_test.dart new file mode 100644 index 000000000..14984bb83 --- /dev/null +++ b/pkgs/test_reflective_loader/test/location_test.dart @@ -0,0 +1,69 @@ +// 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. + +import 'dart:convert'; +import 'dart:io'; +import 'dart:isolate'; + +import 'package:path/path.dart' as path; +import 'package:test/test.dart'; + +void main() { + test("reports correct locations in the JSON output from 'dart test'", + () async { + var testPackagePath = (await Isolate.resolvePackageUri( + Uri.parse('package:test_reflective_loader/')))! + .toFilePath(); + var testFilePath = path.normalize(path.join( + testPackagePath, '..', 'test', 'test_reflective_loader_test.dart')); + var testFileContent = File(testFilePath).readAsLinesSync(); + var result = await Process.run( + Platform.resolvedExecutable, ['test', '-r', 'json', testFilePath]); + + var error = result.stderr.toString().trim(); + var output = result.stdout.toString().trim(); + + expect(error, isEmpty); + expect(output, isNotEmpty); + + for (var event in LineSplitter.split(output).map(jsonDecode)) { + if (event case {'type': 'testStart', 'test': Map test}) { + var name = test['name'] as String; + + // Skip the "loading" test, it never has a location. + if (name.startsWith('loading')) { + continue; + } + + // Split just the method name from the combined test so we can search + // the source code to ensure the locations match up. + name = name.split('|').last.trim(); + + // Expect locations for all remaining fields. + var url = test['url'] as String; + var line = test['line'] as int; + var column = test['column'] as int; + + expect(path.equals(Uri.parse(url).toFilePath(), testFilePath), isTrue); + + // Verify the location provided matches where this test appears in the + // file. + var lineContent = testFileContent[line - 1]; + // If the line is an annotation, skip to the next line + if (lineContent.trim().startsWith('@')) { + lineContent = testFileContent[line]; + } + expect(lineContent, contains(name), + reason: 'JSON reports test $name on line $line, ' + 'but line content is "$lineContent"'); + + // Verify the column too. + var columnContent = lineContent.substring(column - 1); + expect(columnContent, contains(name), + reason: 'JSON reports test $name at column $column, ' + 'but text at column is "$columnContent"'); + } + } + }); +} From dec7d56adce9fe89c63d85694ceb2aae120dbd80 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Tue, 13 May 2025 11:37:52 +0100 Subject: [PATCH 2/6] Reduce what's imported and use same prefix --- .../lib/test_reflective_loader.dart | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pkgs/test_reflective_loader/lib/test_reflective_loader.dart b/pkgs/test_reflective_loader/lib/test_reflective_loader.dart index f2e4928c8..5112b17db 100644 --- a/pkgs/test_reflective_loader/lib/test_reflective_loader.dart +++ b/pkgs/test_reflective_loader/lib/test_reflective_loader.dart @@ -5,7 +5,7 @@ import 'dart:async'; import 'dart:mirrors'; -import 'package:test/scaffolding.dart'; +import 'package:test/scaffolding.dart' as test_package show TestLocation; import 'package:test/test.dart' as test_package; /// A marker annotation used to annotate test methods which are expected to fail @@ -308,14 +308,14 @@ class _AssertFailingTest { class _Group { final bool isSolo; final String name; - final TestLocation? location; + final test_package.TestLocation? location; final List<_Test> tests = <_Test>[]; _Group(this.isSolo, this.name, this.location); bool get hasSoloTest => tests.any((test) => test.isSolo); - void addSkippedTest(String name, TestLocation? location) { + void addSkippedTest(String name, test_package.TestLocation? location) { var fullName = _combineNames(this.name, name); tests.add(_Test.skipped(isSolo, fullName, location)); } @@ -347,7 +347,7 @@ class _Test { final String name; final _TestFunction function; final test_package.Timeout? timeout; - final TestLocation? location; + final test_package.TestLocation? location; final bool isSkipped; @@ -361,9 +361,10 @@ class _Test { } extension on DeclarationMirror { - TestLocation? get testLocation { + test_package.TestLocation? get testLocation { if (location case var location?) { - return TestLocation(location.sourceUri, location.line, location.column); + return test_package.TestLocation( + location.sourceUri, location.line, location.column); } else { return null; } From 05d48c217c3c256756422c48922c9ecab807e2b3 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 21 May 2025 10:01:24 +0100 Subject: [PATCH 3/6] Bump to pkg:test 1.26.1 --- pkgs/test_reflective_loader/CHANGELOG.md | 2 +- pkgs/test_reflective_loader/pubspec.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/test_reflective_loader/CHANGELOG.md b/pkgs/test_reflective_loader/CHANGELOG.md index acecb514a..5e19503fd 100644 --- a/pkgs/test_reflective_loader/CHANGELOG.md +++ b/pkgs/test_reflective_loader/CHANGELOG.md @@ -1,6 +1,6 @@ ## 0.3.0 -- Update to `package:test` 1.26.0. +- Update to `package:test` 1.26.1. - Pass locations of groups/tests to `package:test` to improve locations reported in the JSON reporter that may be used for navigation in IDEs. diff --git a/pkgs/test_reflective_loader/pubspec.yaml b/pkgs/test_reflective_loader/pubspec.yaml index 47da4d8b3..a422207f5 100644 --- a/pkgs/test_reflective_loader/pubspec.yaml +++ b/pkgs/test_reflective_loader/pubspec.yaml @@ -8,7 +8,7 @@ environment: sdk: ^3.1.0 dependencies: - test: ^1.26.0 + test: ^1.26.1 dev_dependencies: dart_flutter_team_lints: ^3.0.0 From 76d3c8fc6fa60435165d55c690eb0b5d96561aa1 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 21 May 2025 19:06:39 +0100 Subject: [PATCH 4/6] Require Dart 3.2 to match the pkg:test dependency --- .github/workflows/test_reflective_loader.yaml | 2 +- pkgs/test_reflective_loader/CHANGELOG.md | 1 + pkgs/test_reflective_loader/pubspec.yaml | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test_reflective_loader.yaml b/.github/workflows/test_reflective_loader.yaml index 7550fff5f..bc71a10d7 100644 --- a/.github/workflows/test_reflective_loader.yaml +++ b/.github/workflows/test_reflective_loader.yaml @@ -26,7 +26,7 @@ jobs: strategy: fail-fast: false matrix: - sdk: [dev, 3.1] + sdk: [dev, 3.2] steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 diff --git a/pkgs/test_reflective_loader/CHANGELOG.md b/pkgs/test_reflective_loader/CHANGELOG.md index 5e19503fd..3fa68ee16 100644 --- a/pkgs/test_reflective_loader/CHANGELOG.md +++ b/pkgs/test_reflective_loader/CHANGELOG.md @@ -1,5 +1,6 @@ ## 0.3.0 +- Require Dart `^3.2.0`. - Update to `package:test` 1.26.1. - Pass locations of groups/tests to `package:test` to improve locations reported in the JSON reporter that may be used for navigation in IDEs. diff --git a/pkgs/test_reflective_loader/pubspec.yaml b/pkgs/test_reflective_loader/pubspec.yaml index a422207f5..b0f79d2f1 100644 --- a/pkgs/test_reflective_loader/pubspec.yaml +++ b/pkgs/test_reflective_loader/pubspec.yaml @@ -5,7 +5,7 @@ repository: https://github.com/dart-lang/tools/tree/main/pkgs/test_reflective_lo issue_tracker: https://github.com/dart-lang/tools/labels/package%3Atest_reflective_loader environment: - sdk: ^3.1.0 + sdk: ^3.2.0 dependencies: test: ^1.26.1 From 67fe3ae9cbee1f00380617e4ea0f1665abec0664 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 21 May 2025 19:54:14 +0100 Subject: [PATCH 5/6] Bump to SDK 3.5 --- .github/workflows/test_reflective_loader.yaml | 2 +- pkgs/test_reflective_loader/CHANGELOG.md | 2 +- pkgs/test_reflective_loader/pubspec.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test_reflective_loader.yaml b/.github/workflows/test_reflective_loader.yaml index bc71a10d7..8e70b85a3 100644 --- a/.github/workflows/test_reflective_loader.yaml +++ b/.github/workflows/test_reflective_loader.yaml @@ -26,7 +26,7 @@ jobs: strategy: fail-fast: false matrix: - sdk: [dev, 3.2] + sdk: [dev, 3.5] steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 diff --git a/pkgs/test_reflective_loader/CHANGELOG.md b/pkgs/test_reflective_loader/CHANGELOG.md index 3fa68ee16..61ba81bde 100644 --- a/pkgs/test_reflective_loader/CHANGELOG.md +++ b/pkgs/test_reflective_loader/CHANGELOG.md @@ -1,6 +1,6 @@ ## 0.3.0 -- Require Dart `^3.2.0`. +- Require Dart `^3.5.0`. - Update to `package:test` 1.26.1. - Pass locations of groups/tests to `package:test` to improve locations reported in the JSON reporter that may be used for navigation in IDEs. diff --git a/pkgs/test_reflective_loader/pubspec.yaml b/pkgs/test_reflective_loader/pubspec.yaml index b0f79d2f1..262a3498f 100644 --- a/pkgs/test_reflective_loader/pubspec.yaml +++ b/pkgs/test_reflective_loader/pubspec.yaml @@ -5,7 +5,7 @@ repository: https://github.com/dart-lang/tools/tree/main/pkgs/test_reflective_lo issue_tracker: https://github.com/dart-lang/tools/labels/package%3Atest_reflective_loader environment: - sdk: ^3.2.0 + sdk: ^3.5.0 dependencies: test: ^1.26.1 From fe822946610ca8abc6eb469a0657db252fc0e95f Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Thu, 22 May 2025 10:49:47 +0100 Subject: [PATCH 6/6] Remove unnecessary import --- pkgs/test_reflective_loader/lib/test_reflective_loader.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/pkgs/test_reflective_loader/lib/test_reflective_loader.dart b/pkgs/test_reflective_loader/lib/test_reflective_loader.dart index 5112b17db..9c3a103ce 100644 --- a/pkgs/test_reflective_loader/lib/test_reflective_loader.dart +++ b/pkgs/test_reflective_loader/lib/test_reflective_loader.dart @@ -5,7 +5,6 @@ import 'dart:async'; import 'dart:mirrors'; -import 'package:test/scaffolding.dart' as test_package show TestLocation; import 'package:test/test.dart' as test_package; /// A marker annotation used to annotate test methods which are expected to fail