SDK tests with test_reflective_loader get out of sync with gutter icons if new tests are added after running once

Describe the bug
When working with the SDK and test_reflective_loader tests, adding a new test after running the suite makes some tests get out of sync with the gutter.

To Reproduce
Steps to reproduce the behavior:

Open pkg/linter/test/rules/prefer_initializing_formals_test.dart (or whatever test file with it)
Run the tests
Add test_a() {} above all other tests
Run again, see the test below it get out of sync with the test gutter

Expected behavior
For the tests to have different gutter icons.

Screenshots

Please complete the following information:

Workspace Environment

Dart Code extension: 3.117.20250813
Flutter extension: 3.117.20250731 (activated)

App: Visual Studio Code
App Host: desktop
Version: win 1.103.2

Workspace type: Dart (LSP) (Multiroot)
Workspace name: sdk (Workspace)

Dart (3.10.0-edge.1a2411efc9a5988d76b967202a99bb9e38f2cbfc): C:\Users\felip_0vh5fa6\dart
Flutter (3.32.8): C:\Users\felip_0vh5fa6\.puro\envs\stable\flutter (No device)

Output from 'dart info'

C:\Users\felip_0vh5fa6\dart\bin\dart.exe info

If providing this information as part of reporting a bug, please review the information
below to ensure it only contains things you're comfortable posting publicly.

General info

Dart 3.10.0-edge.1a2411efc9a5988d76b967202a99bb9e38f2cbfc (main) (Fri Aug 22 04:22:13 2025 -0700) on "windows_x64"
on windows / "Windows 11 Pro" 10.0 (Build 26100)
locale is pt-BR

Process info

Memory	CPU	Elapsed time	Command line
0 MB	--		dart.exe
0 MB	--		dart.exe
0 MB	--		dart.exe
0 MB	--		dart.exe
0 MB	--		dart.exe
0 MB	--		dart.exe
0 MB	--		dart.exe
0 MB	--		dart.exe
0 MB	--		dart.exe
0 MB	--		dart.exe
0 MB	--		dart.exe
0 MB	--		dart.exe

Thanks, I can repro this with the exact steps above. Feels a bit like #5544, but that should be solved. I'll have to do some digging.

A smaller repro:

import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';

void main() {
  defineReflectiveSuite(() {
    defineReflectiveTests(FooTests);
  });
}

@reflectiveTest
class FooTests {
  test_bar() {
    expect(1, 1);
  }

  test_baz() {
    expect(1, 1);
  }
}

Add test_a() {} as the first test, then re-run as described above and test_bar gets merged into it because it seems to use the original line number.

I tracked down the bug and implemented a fix, but it reintroduced something very similar to #5544! After some digging, it seems like VS Code has a very similar bug to what we have, so I'm not hitting that.

I've opened microsoft/vscode#264581 and will hold off committing anything for this until that's been fixed (or responded to) because I think it would be very difficult for us to fix without that fixed first.

WIP - CONTAINS DEBUG LOGGING

Notes for when I get back to this (since I don't know how quickly it'll be resolved):

I've pushed the code that I think should work (NOTE: INCLUDES DEBUG console.warns!) to this branch:

https://github.com/Dart-Code/Dart-Code/tree/issue5668

It adds a rangeIsDirty flag to track when we update the range, to ensure we only send it to VS Code when it's been updated (to avoid #5544 coming back, where we accidentally snap a test that was tracked as moved by VS Code back to its original location).

However when testing with a mix of TRL and normal tests, duplicating the line with the empty comment causes the TRL tests to move back to their "original" line numbers (this test project is at D:\Dev\Test Projects\dartcode_5668\test\dartcode_5668_test.dart):

import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';

void main() {
  defineReflectiveSuite(() {
    defineReflectiveTests(FooTests);
  });

  group('foo', () {
    test('danny', () {
      //
      //
      //
      //
      print('test');
    });
  });
}

@reflectiveTest
class FooTests {
  test_a() {}

  test_bar() {
    expect(1, 1);
  }

  test_baz() {
    expect(1, 1);
  }
}

bad_tests.mp4

@bwilkerson

Given the response on the VS Code issue (by design, even though I don't feel I understand why they do this), I think there are a few options:

We try to reuse our offset tracker (used by the experimental UI guidelines) to track where the test_reflective_loader tests move to in our model and sync them back to VS Code. This will probably work but since that offset tracker is only currently used by an experimental feature (and one where we quickly get an Outline and replace the tracked positions anyway) it's not had a huge amount of testing.
We do something to just avoid registering test locations for any test results if there are also Outline-based tests in the same file. It's probably unlikely that any test file has a mix of test_reflective_loader tests and standard tests that show up in the outline, however I'm not certain that there aren't cases where we rely on the test-results for test locations for standard tests (for example those that are dynamically created inside loops or with variables in their names)
We update the analysis server to understand test_reflective_loader tests and include them in the outline in the same way as other tests. This way when a file changes, we will recompute their locations too instead of only having their stale positions from the last test run.

I feel like (3) probably would result in the best experience, but it also feels like the most work (server would have to learn about @reflectiveTest - or some new generic equivalent of and then make assumptions that match the logic in pkg:test_reflective_loader for which methods are tests). My feeling is that test_reflective_loader isn't a particularly commonly used package so I don't know if this is a good trade-off compared to one of the less-good-but-simpler options. (cc @bwilkerson in case you have any opinions on this).

We update the analysis server to understand test_reflective_loader tests and include them in the outline in the same way as other tests.

I don't understand what this means. Which 'outline', and what does "the same way" mean?

I don't understand what this means. Which 'outline', and what does "the same way" mean?

Sorry, I didn't give much context there 😄

The analysis_server currently includes nodes in the Outline (etc. analysis.outline notifications in the legacy protocol, and we provide the same data structure over LSP) for group() and test() calls (UNIT_TEST_GROUP and UNIT_TEST_TEST). We use these in Dart-Code to discover tests and report them to VS Code (so that it can populate the test runner).

However the server does not produce outline nodes with these types for methods that represent tests that use package:test_reflective_loader) and therefore Dart-Code does not discover these tests. Instead, when the tests are run and we get back the JSON data from pkg:test, we then report those results to VS Code.

This means that if you run some pkg:test_reflective_loader tests and then modify the file, Dart-Code has lost track of where those tests are until you next run them. VS Code generally tracks them fine, but under some circumstances will revert back to the location we hold, which may be stale.

If the analysis_server could include information about the positions of these tests in some form (similar to, or the same as, UNIT_TEST_TEST) then Dart-Code could track them the same way as normal tests (which use the @isTest and @isTestGroup annotations) when the file is being modified.

Ok, thanks.

We can't use the annotations to support this; they have to be placed on a function that's being invoked at a given location, and we're declaring a method, not invoking a function.

But I'd have no problem with adding explicit support for a package we control. We do create nodes for the methods, we just don't label them as UNIT_TEST_TEST. It would be easy enough to change the label we're using, though I don't know what impact it would have on the UI. It doesn't feel like much work to me, but maybe I'm forgetting something that would make it hard.

Would we also want to associate UNIT_TEST_GROUP with the class?

Would we also want to associate UNIT_TEST_GROUP with the class?

Yes, I think this would make sense - although we might need to resolve dart-lang/tools#2107 because today test_reflective_loader doesn't actually produce groups in the results, and we should probably ensure that the tests/groups statically match those we have at runtime (we'd want to be able to click "Run" on one of these groups/tests and have it actually work).

It doesn't feel like much work to me, but maybe I'm forgetting something that would make it hard.

I also don't think it will be much work, my main concern was about whether we think it's a good fit for server (handling this package specifically, and possible duplicating some of the logic about what methods are tests that normally live in that package).

That said, if we change the ElementKind of these classes/methods from CLASS/METHOD to UNIT_TEST_GROUP/UNIT_TEST_TEST I don't know if that would negatively affect legacy protocol clients. If so, we could nest new nodes of these kinds inside those (although that may also have the possibility of upsetting things)? 🤔

@jwren

The legacy protocol supports the UNIT_TEST_* kinds, but it's possible that there are context menu items driven by the kind.

@jwren @pq In case they know or can find out.

I do worry slightly about getting the logic for this right in server [...], there could be a mismatch for users of the older version.

Do you mean in the position of the CodeLenses? Wouldn't/Couldn't we guard that by version? Although I would be fine with things working differently between versions, I somewhat expect that hahaha.

Do you mean in the position of the CodeLenses?

Sorry no, I mean a mismatch between what Dart-Code computes as the hierarchy for a the test ("group" > "test") and what the results when running the tests (with an older test_reflective_loader) come through as (a single top-level test named "group - test").

In Dart-Code, we need to merge tests discovered statically (from the server outline data) and the results that come through so you get a single test tree that can include tests that haven't been run, but show the status when they have been run.

Unfortunately we can never be 100% reliable, because you can do something like this:

test('test ${DateTime.now().millisecondsSinceEpoch}', () {});

So we have to make some assumptions when trying to match them up (I think we allow names to differ where we see variable names. but I don't think we handle a different hierarchy).

Add (skipped) failing test for #5668

Add an experimental flag for custom test tracking

@FMorschel

I've added some new tracking for test locations that should improve things for tests the server doesn't report (both pkg:test_reflective_loader, and other dynamic tests, such as those with variables in their names that we previously had other workarounds for).

I'd like to have some more testing of this before enabling it by default so it's currently behind a flag dart.experimentalTestTracking. I've enabled it and will keep an eye out when working in projects using test_reflective_loader and if all goes well I'll enable it for the next release.

@FMorschel you might also want to enable it and keep an eye out for issues. I'm publishing a pre-release extension shortly that includes it in advance of the stable release that's likely to happen tomorrow or Wednesday.

(Edit: Published v3.119.20250929 - usually they take 20mins or so to show up in VS Code)

@FMorschel

@FMorschel I haven't seen any issues with this setting on, so I'm considering changing this flag to default true for the upcoming release (so it's still possible to revert in case of any serious issues), and removing it after the release.

Besides #5735, have you noticed any issues that might make me want to delay that?

I've also seen (albeit once - we discussed on Discord briefly) a problem with the gutter staying correctly, but the "expected" value VS Code shows being moved up some lines, but I've not yet been able to reproduce this again (less so reliably).

Even if I can repro and open an issue, I don't think it would be bad to have this enabled. I'd say to go ahead!

sgtm, thanks!

Navigation Menu

Repository navigation

SDK tests with `test_reflective_loader` get out of sync with gutter icons if new tests are added after running once #5668

General info

Process info

3 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

SDK tests with test_reflective_loader get out of sync with gutter icons if new tests are added after running once #5668

Description

General info

Process info

Activity

DanTup commented on Sep 2, 2025

DanTup commented on Sep 2, 2025

DanTup commented on Sep 2, 2025

DanTup commented on Sep 2, 2025

DanTup commented on Sep 3, 2025

bwilkerson commented on Sep 3, 2025

DanTup commented on Sep 3, 2025

bwilkerson commented on Sep 3, 2025

DanTup commented on Sep 3, 2025

bwilkerson commented on Sep 3, 2025

3 remaining items

FMorschel commented on Sep 23, 2025

DanTup commented on Sep 23, 2025

DanTup commented on Sep 29, 2025

DanTup commented on Oct 29, 2025

FMorschel commented on Oct 29, 2025

DanTup commented on Oct 29, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

SDK tests with `test_reflective_loader` get out of sync with gutter icons if new tests are added after running once #5668