[test_reflective_loader] Don't add setUp/tearDown if not defined, and provide correct locations - #2341
Conversation
… provide correct locations This makes the calls to `setUpAll` and `tearDownAll` conditional based on whether there are corresponding methods, so that there aren't phantom nodes in the IDE for all test classes. It also adds missing locations, so that they navigate to the correct location when clicked. Fixes Dart-Code/Dart-Code#5967 Fixes Dart-Code/Dart-Code#5950 While testingh, I also noticed package:test is always adding a tearDownAll if there is a setUpAll - I've filed dart-lang/test#2608 about that.
|
(cc @scheglov, this is low priority but I don't know if you get any notifications here since it's a mono repo) |
There was a problem hiding this comment.
Code Review
This pull request is a great improvement. It conditionally adds setUpAll and tearDownAll calls, preventing phantom nodes in IDEs, and also provides correct locations for them. The refactoring of test running logic into a shared utils.dart file is a nice cleanup. The changes are well-tested. I've left a few minor comments regarding documentation to align with the project's style guide.
|
@scheglov I think this is waiting for your review :) |
|
@scheglov ping :-) |
|
@scheglov thanks! Do you know what's with the bots? There are two actions that seem to be waiting (GH Actions Scan and Workflow Security Scan) but since it's been 18 hours since this was approved, I'm wondering if they are stuck. |
|
Yeah, I was waiting for bots, in case if they will start later; not landing yet. |
Revisions updated by `dart tools/rev_sdk_deps.dart`. core (https://github.com/dart-lang/core/compare/fe516ee..4a5ae2b): 4a5ae2bc Wed Aug 5 15:49:08 2026 +0000 dependabot[bot] Bump the github-actions group with 4 updates (dart-lang/core#988) 30363b90 Wed Aug 5 17:37:07 2026 +0200 Moritz Add skip-changelog-check label to dependabot PRs (dart-lang/core#987) ef697b97 Wed Aug 5 08:23:09 2026 +0200 dependabot[bot] Bump the github-actions group across 1 directory with 3 updates (dart-lang/core#983) 95bf1b9d Wed Aug 5 08:21:09 2026 +0200 Moritz Add pub dependabot support (dart-lang/core#986) b0400198 Tue Aug 4 18:48:31 2026 +0200 Moritz Replace permissions: read-all with permissions: contents: read in workflows (dart-lang/core#985) e1b0a677 Mon Aug 3 17:51:19 2026 +0200 Moritz Pin actions/checkout (dart-lang/core#984) dartdoc (https://github.com/dart-lang/dartdoc/compare/ac96918..1fd02f7): 1fd02f73 Sat Aug 1 18:52:23 2026 +0000 dependabot[bot] Bump the github-actions group with 3 updates (dart-lang/dartdoc#4276) ecosystem (https://github.com/dart-lang/ecosystem/compare/edfdb3b..ed9c592): ed9c592 Wed Aug 5 15:19:24 2026 +0200 Sigurd Meldgaard Support pull requests from forks and custom branches in CI workflows (dart-lang/ecosystem#440) d8c8a4c Wed Aug 5 13:09:39 2026 +0200 Sigurd Meldgaard Support configurable tag prefix in firehose and publish.yaml (dart-lang/ecosystem#436) http (https://github.com/dart-lang/http/compare/5d94ef5..9cb80c7): 9cb80c7 Wed Aug 5 16:32:40 2026 -0700 Nate Bosch Undeprecate the withCredentials field (dart-lang/http#1962) 44496f4 Mon Aug 3 20:58:34 2026 +0300 Melih Arık doc(http_profile): fix broken code examples in header setter docs (dart-lang/http#1954) 49ddf11 Mon Jul 20 21:08:18 2026 +0300 Ellet docs(readme): add close method override in UserAgentClient example (dart-lang/http#1949) fe4aaa9 Tue Jul 14 11:25:42 2026 -0700 Nate Bosch Switch to feature version bump (dart-lang/http#1953) 664fe06 Mon Jul 13 22:08:59 2026 +0200 Maurice Raguse Support for configuring the browser `fetch` credentials mode in `BrowserClient`. (dart-lang/http#1937) shelf (https://github.com/dart-lang/shelf/compare/6918a76..fb3f931): fb3f931 Sat Aug 1 20:01:06 2026 +0000 dependabot[bot] Bump the github-actions group with 4 updates (dart-lang/shelf#537) test (https://github.com/dart-lang/test/compare/4838365..dd426d4): dd426d43 Tue Aug 4 15:46:52 2026 -0700 Nate Bosch Obfuscate the test runner connection for VM and Node tests (dart-lang/test#2704) tools (https://github.com/dart-lang/tools/compare/b827a6e..f3ec9ed): f3ec9ed5 Wed Aug 5 16:36:58 2026 +0000 dependabot[bot] Bump the github-actions group across 1 directory with 8 updates (dart-lang/tools#2496) 6dac7b1f Wed Aug 5 17:37:23 2026 +0200 Moritz Add skip-changelog-check label to dependabot PRs (dart-lang/tools#2495) f16493da Wed Aug 5 08:11:23 2026 +0000 dependabot[bot] Bump analyzer from 10.2.0 to 14.1.0 in /pkgs/graphs (dart-lang/tools#2493) 7f33666d Wed Aug 5 08:21:24 2026 +0200 Moritz Add pub dependabot support (dart-lang/tools#2492) 1e3aac17 Tue Aug 4 18:52:43 2026 +0200 Moritz Replace permissions: read-all with permissions: contents: read in workflows (dart-lang/tools#2490) e79e9cc4 Tue Aug 4 08:47:01 2026 -0300 Felipe Morschel [yaml] Better recovering for list entry without `- ` prefix (dart-lang/tools#2427) ca89367d Mon Aug 3 17:50:48 2026 +0200 Moritz ci: pin unpinned GitHub Actions references to commit SHAs (dart-lang/tools#2487) 0da8d930 Mon Aug 3 10:48:16 2026 +0200 Moritz Update CODEOWNERS with global fallback and missing packages (dart-lang/tools#2469) 28396db2 Mon Aug 3 09:34:24 2026 +0200 Moritz [test_reflective_loader] Ignore unreachable_from_main in test files (dart-lang/tools#2486) 168a6e13 Mon Aug 3 08:28:27 2026 +0200 Morgan :) source_maps: check bounds earlier. (dart-lang/tools#2483) 27756a9b Fri Jul 31 17:37:37 2026 +0100 Danny Tuppeny [test_reflective_loader] Don't add setUp/tearDown if not defined, and provide correct locations (dart-lang/tools#2341) web (https://github.com/dart-lang/web/compare/12a9ca2..f1b9d56): f1b9d56 Sat Aug 1 19:43:21 2026 +0000 dependabot[bot] Bump the github-actions group with 3 updates (dart-lang/web#566) R=bquinlan@google.com Change-Id: Id362bbe87d302cfb6a1b6618de12c67be72f1855 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/532380 Commit-Queue: Nate Bosch <nbosch@google.com> Reviewed-by: Brian Quinlan <bquinlan@google.com>
This makes the calls to
setUpAllandtearDownAllconditional based on whether there are corresponding methods, so that there aren't phantom nodes in the IDE for all test classes.It also adds missing locations, so that they navigate to the correct location when clicked.
Fixes Dart-Code/Dart-Code#5967
Fixes Dart-Code/Dart-Code#5950
While testing, I also noticed
package:testis always adding atearDownAllif there is asetUpAll- I've filed dart-lang/test#2608 about that.