diff --git a/pkgs/test_reflective_loader/CHANGELOG.md b/pkgs/test_reflective_loader/CHANGELOG.md index 61ba81bde..a4270cf48 100644 --- a/pkgs/test_reflective_loader/CHANGELOG.md +++ b/pkgs/test_reflective_loader/CHANGELOG.md @@ -1,3 +1,10 @@ +## 0.4.0 + +- Test classes and suites with `name`s are now registered with `package:test`s + `group()` function to produce a hierarchy of groups/tests rather than a flat + set of tests with concatenated names. This may improve the display of tests + in IDEs test explorers. + ## 0.3.0 - Require Dart `^3.5.0`. diff --git a/pkgs/test_reflective_loader/lib/test_reflective_loader.dart b/pkgs/test_reflective_loader/lib/test_reflective_loader.dart index 9c3a103ce..65b3e1041 100644 --- a/pkgs/test_reflective_loader/lib/test_reflective_loader.dart +++ b/pkgs/test_reflective_loader/lib/test_reflective_loader.dart @@ -25,9 +25,12 @@ const Object skippedTest = SkippedTest(); /// A marker annotation used to annotate "solo" groups and tests. const Object soloTest = _SoloTest(); -final List<_Group> _currentGroups = <_Group>[]; -int _currentSuiteLevel = 0; -String _currentSuiteName = ''; +/// The current group stack of nested [defineReflectiveSuite] calls. +List<_Group> _currentGroupStack = []; + +/// The root groups or tests created by [defineReflectiveSuite] or +/// [defineReflectiveTests] calls. +List<_GroupEntry> _rootGroupEntries = []; /// Is `true` the application is running in the checked mode. final bool _isCheckedMode = () { @@ -43,16 +46,9 @@ final bool _isCheckedMode = () { /// add normal and "solo" tests, and also calls [defineReflectiveSuite] to /// create embedded suites. If the current suite is the top-level one, perform /// check for "solo" groups and tests, and run all or only "solo" items. -void defineReflectiveSuite(void Function() define, {String name = ''}) { - var groupName = _currentSuiteName; - _currentSuiteLevel++; - try { - _currentSuiteName = _combineNames(_currentSuiteName, name); - define(); - } finally { - _currentSuiteName = groupName; - _currentSuiteLevel--; - } +void defineReflectiveSuite(void Function() define, {String? name}) { + _addGroup(_Group(name), define); + _addTestsIfTopLevelSuite(); } @@ -83,108 +79,90 @@ void defineReflectiveTests(Type type) { 'in order to be run by runReflectiveTests.'); } - _Group group; - { - var isSolo = _hasAnnotationInstance(classMirror, soloTest); - var className = MirrorSystem.getName(classMirror.simpleName); - group = _Group(isSolo, _combineNames(_currentSuiteName, className), - classMirror.testLocation); - _currentGroups.add(group); - } + var isSolo = _hasAnnotationInstance(classMirror, soloTest); + var className = MirrorSystem.getName(classMirror.simpleName); - classMirror.instanceMembers - .forEach((Symbol symbol, MethodMirror memberMirror) { - // we need only methods - if (!memberMirror.isRegularMethod) { - return; - } - // prepare information about the method - var memberName = MirrorSystem.getName(symbol); - var isSolo = memberName.startsWith('solo_') || - _hasAnnotationInstance(memberMirror, soloTest); - // test_ - if (memberName.startsWith('test_')) { - if (_hasSkippedTestAnnotation(memberMirror)) { - group.addSkippedTest(memberName, memberMirror.testLocation); - } else { - group.addTest(isSolo, memberName, memberMirror, () { - if (_hasFailingTestAnnotation(memberMirror) || - _isCheckedMode && _hasAssertFailingTestAnnotation(memberMirror)) { - return _runFailingTest(classMirror, symbol); - } else { - return _runTest(classMirror, symbol); - } - }); + _addGroup(_Group(className, solo: isSolo, location: classMirror.testLocation), + () { + classMirror.instanceMembers + .forEach((Symbol symbol, MethodMirror memberMirror) { + // we need only methods + if (!memberMirror.isRegularMethod) { + return; } - return; - } - // solo_test_ - if (memberName.startsWith('solo_test_')) { - group.addTest(true, memberName, memberMirror, () { - return _runTest(classMirror, symbol); - }); - } - // fail_test_ - if (memberName.startsWith('fail_')) { - group.addTest(isSolo, memberName, memberMirror, () { - return _runFailingTest(classMirror, symbol); - }); - } - // solo_fail_test_ - if (memberName.startsWith('solo_fail_')) { - group.addTest(true, memberName, memberMirror, () { - return _runFailingTest(classMirror, symbol); - }); - } - // skip_test_ - if (memberName.startsWith('skip_test_')) { - group.addSkippedTest(memberName, memberMirror.testLocation); - } + // prepare information about the method + var memberName = MirrorSystem.getName(symbol); + var isTest = memberName.startsWith(RegExp('(solo_|fail_|skip_)*test_')); + if (isTest) { + var isSolo = memberName.startsWith('solo_') || + _hasAnnotationInstance(memberMirror, soloTest); + var isSkipped = memberName.startsWith('skip_') || + _hasSkippedTestAnnotation(memberMirror); + var expectFail = memberName.startsWith('fail_') || + memberName.startsWith('solo_fail_') || + _hasFailingTestAnnotation(memberMirror) || + _isCheckedMode && _hasAssertFailingTestAnnotation(memberMirror); + var timeout = + _getAnnotationInstance(memberMirror, TestTimeout) as TestTimeout?; + + _addTest( + _Test( + memberName, + timeout: timeout?._timeout, + location: memberMirror.testLocation, + solo: isSolo, + skip: isSkipped, + () => expectFail + ? _runFailingTest(classMirror, symbol) + : _runTest(classMirror, symbol)), + ); + } + }); }); - // Support for the case of missing enclosing [defineReflectiveSuite]. _addTestsIfTopLevelSuite(); } -/// If the current suite is the top-level one, add tests to the `test` package. +/// If we're back at the top level ([_currentGroupStack] is empty), registers +/// all known groups and tests by calling [test_package.group] and +/// [test_package.test] appropriately. void _addTestsIfTopLevelSuite() { - if (_currentSuiteLevel == 0) { - void runTests({required bool allGroups, required bool allTests}) { - for (var group in _currentGroups) { - if (allGroups || group.isSolo) { - for (var test in group.tests) { - if (allTests || test.isSolo) { - test_package.test(test.name, test.function, - timeout: test.timeout, - skip: test.isSkipped, - location: test.location); - } + if (_currentGroupStack.isNotEmpty) return; + + void addGroupsAndTests(List<_GroupEntry> entries) { + for (var entry in entries) { + switch (entry) { + case _Group group: + // Only add groups if they have names, otherwise just add their + // children directly. + if (group.name != null) { + test_package.group( + group.name, + location: group.location, + // ignore: deprecated_member_use, invalid_use_of_do_not_submit_member + solo: group.solo, + () => addGroupsAndTests(group.children), + ); + } else { + addGroupsAndTests(group.children); } - } + break; + case _Test test: + test_package.test( + test.name, + timeout: test.timeout, + location: test.location, + // ignore: deprecated_member_use, invalid_use_of_do_not_submit_member + solo: test.solo, + skip: test.skip, + test.function); + break; } } - - if (_currentGroups.any((g) => g.hasSoloTest)) { - runTests(allGroups: true, allTests: false); - } else if (_currentGroups.any((g) => g.isSolo)) { - runTests(allGroups: false, allTests: true); - } else { - runTests(allGroups: true, allTests: true); - } - _currentGroups.clear(); } -} -/// Return the combination of the [base] and [addition] names. -/// If any other two is `null`, then the other one is returned. -String _combineNames(String base, String addition) { - if (base.isEmpty) { - return addition; - } else if (addition.isEmpty) { - return base; - } else { - return '$base | $addition'; - } + addGroupsAndTests(_rootGroupEntries); + _rootGroupEntries.clear(); } Object? _getAnnotationInstance(DeclarationMirror declaration, Type type) { @@ -226,6 +204,28 @@ Future _invokeSymbolIfExists( return Future.value(invocationResult); } +/// Adds a group to the current stack and executes [define] for child group +/// or tests definitions. +void _addGroup(_Group group, void Function() define) { + var parentCollection = + _currentGroupStack.lastOrNull?.children ?? _rootGroupEntries; + parentCollection.add(group); + _currentGroupStack.add(group); + try { + define(); + } finally { + _currentGroupStack.removeLast(); + } +} + +/// Adds a test to the current group (or as a root test if there is no current +/// group). +void _addTest(_Test test) { + var parentCollection = + _currentGroupStack.lastOrNull?.children ?? _rootGroupEntries; + parentCollection.add(test); +} + /// Run a test that is expected to fail, and confirm that it fails. /// /// This properly handles the following cases: @@ -267,8 +267,6 @@ Future _runTest(ClassMirror classMirror, Symbol symbol) async { } } -typedef _TestFunction = dynamic Function(); - /// A marker annotation used to annotate test methods which are expected to /// fail. class FailingTest { @@ -303,32 +301,6 @@ class _AssertFailingTest { const _AssertFailingTest(); } -/// Information about a type based test group. -class _Group { - final bool isSolo; - final String name; - 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, test_package.TestLocation? location) { - var fullName = _combineNames(this.name, name); - tests.add(_Test.skipped(isSolo, fullName, location)); - } - - void addTest(bool isSolo, String name, MethodMirror memberMirror, - _TestFunction function) { - var fullName = _combineNames(this.name, name); - var timeout = - _getAnnotationInstance(memberMirror, TestTimeout) as TestTimeout?; - tests.add(_Test(isSolo, fullName, function, timeout?._timeout, - memberMirror.testLocation)); - } -} - /// A marker annotation used to instruct dart2js to keep reflection information /// for the annotated classes. class _ReflectiveTest { @@ -340,23 +312,45 @@ class _SoloTest { const _SoloTest(); } -/// Information about a test. -class _Test { - final bool isSolo; - final String name; - final _TestFunction function; - final test_package.Timeout? timeout; +abstract class _GroupEntry { + final String? name; final test_package.TestLocation? location; + final bool solo; + + _GroupEntry( + this.name, { + this.location, + this.solo = false, + }); +} + +/// Information about a test group which could be from a call to +/// [defineReflectiveSuite] with a `name`, or a test class itself. +class _Group extends _GroupEntry { + final List<_GroupEntry> children = []; - final bool isSkipped; + _Group( + super.name, { + super.location, + super.solo, + }); +} - _Test(this.isSolo, this.name, this.function, this.timeout, this.location) - : isSkipped = false; +/// Information about a test created for a method of a class with +/// [defineReflectiveTests]. +class _Test extends _GroupEntry { + final FutureOr? Function() function; + final bool skip; + final test_package.Timeout? timeout; - _Test.skipped(this.isSolo, this.name, this.location) - : isSkipped = true, - function = (() {}), - timeout = null; + _Test( + super.name, + this.function, { + required super.location, + required super.solo, + required this.skip, + required this.timeout, + }); } extension on DeclarationMirror { diff --git a/pkgs/test_reflective_loader/pubspec.yaml b/pkgs/test_reflective_loader/pubspec.yaml index 262a3498f..efdec89d1 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.3.0 +version: 0.4.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 diff --git a/pkgs/test_reflective_loader/test/hierarchy_test.dart b/pkgs/test_reflective_loader/test/hierarchy_test.dart new file mode 100644 index 000000000..c052dd474 --- /dev/null +++ b/pkgs/test_reflective_loader/test/hierarchy_test.dart @@ -0,0 +1,34 @@ +// 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:io'; +import 'dart:isolate'; + +import 'package:path/path.dart' as path; +import 'package:test/test.dart'; + +void main() { + test('builds the correct hierarchy of group names / test names', () async { + var testPackagePath = (await Isolate.resolvePackageUri( + Uri.parse('package:test_reflective_loader/')))! + .toFilePath(); + var testFilePath = path.normalize( + path.join(testPackagePath, '..', 'test', 'hierarchy_test.data.dart')); + var result = + await Process.run(Platform.resolvedExecutable, ['test', testFilePath]); + + var error = result.stderr.toString().trim(); + var output = result.stdout.toString().trim(); + + expect(error, isEmpty); + expect( + output, + allOf([ + contains('+0: SimpleTest test_foo'), + contains('+1: level_1.1 level_2.1 SimpleTest test_foo'), + contains('+2: level_1.1 level_2.2 SimpleTest test_foo'), + contains('+3: All tests passed!'), + ])); + }); +} diff --git a/pkgs/test_reflective_loader/test/hierarchy_test.data.dart b/pkgs/test_reflective_loader/test/hierarchy_test.data.dart new file mode 100644 index 000000000..ae3d96791 --- /dev/null +++ b/pkgs/test_reflective_loader/test/hierarchy_test.data.dart @@ -0,0 +1,32 @@ +// 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 'package:test/test.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +/// This file is not intended to be run directly, but is run by +/// `hierarchy_test.dart`. +void main() { + defineReflectiveTests(SimpleTest); // Ungrouped tests + defineReflectiveSuite(() { + defineReflectiveSuite(() { + defineReflectiveSuite( + () { + defineReflectiveTests(SimpleTest); + }, /* unnamed */ + ); + }, name: 'level_2.1'); + defineReflectiveSuite(() { + defineReflectiveTests(SimpleTest); + }, name: 'level_2.2'); + }, name: 'level_1.1'); +} + +@reflectiveTest +class SimpleTest { + // ignore: non_constant_identifier_names + void test_foo() { + expect(1, 1); + } +} diff --git a/pkgs/test_reflective_loader/test/location_test.dart b/pkgs/test_reflective_loader/test/location_test.dart index 14984bb83..b66b13eda 100644 --- a/pkgs/test_reflective_loader/test/location_test.dart +++ b/pkgs/test_reflective_loader/test/location_test.dart @@ -38,7 +38,7 @@ void main() { // 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(); + name = name.split(' ').last.trim(); // Expect locations for all remaining fields. var url = test['url'] as String;