Gutter test icons sometimes show on wrong line when helper functions call setup/teardown

[osaxma]

Describe the bug
The bug usually occurs while creating new tests, moving them around or grouping them.. See screenshot below.

Reloading VS Code seems the only option atm to realign the buttons (closing/reopening the file does not help)

To Reproduce
Unable to reproduce at the moment. I've seen it multiple time

Expected behavior
The buttons should be aligned with tests.

Screenshots

Please complete the following information:

• Operating System and version: MacOS
• VS Code version: 1.71.2
• Dart extension version: v3.48.4
• Dart/Flutter SDK version: Dart 2.18

Additional Context
When I observed the issues, I had "editor.codeLens": false" in the VS Code settings. I've enabled the code lens now and I'll follow up with an update if the issue appears.

Related Discord discussion (here)

[DanTup]

I was able to reproduce something similar by cutting the entire file to clipboard and then pasting it back. Sometimes it updates fine, and sometimes it does not. When it does not, closing the file and re-opening it seems to fix it. Changing the name of a test also seems to fix it.

[DanTup]

Maye something to do with debouncing? If I cut the entire doc and wait for the test runner to clear, then paste, things come back correctly. If I paste before it updates, then it gets into the broken state.

[DanTup]

I think this might be a VS Code issue. I've filed microsoft/vscode#161320 with some steps to repro what seems like this issue using their sample project.

I believe VS Code's test runner is doing an internal diff and skipping some work when the tests have not really changed. However, because of our debouncing, it's easy to make a set of changes where the test ends up back on the same line as it was originally (and thus doesn't get updated), but VS Code's editor had moved the gutter icons to compensate for some change in-between.

[DanTup]

A fix has been made in VS Code: microsoft/vscode#161320. I'm not certain that it's the same issue, but I think there's a good chance. VS Code usually releases in the first week of each month, so it's probably ~2weeks away. Please let me know if you see this at all again after VS Code updates and we can do some more digging.

[osaxma]

Hi @DanTup

Thanks for your effort in looking into this.

I haven't faced the issue again so far -- probably because I turned "editor.codeLens" to true (thanks again for that awesome tip).

I'll circle back if it appears again.

[DanTup]

I had a quick scan through the code again and I suspect the CodeLens setting doesn't actually impact this now. Our test discovery works even without the CodeLens (which is show we populate the test runner even when codeLens is not enabled).

I'll keep this open a little until the VS Code release goes out and if it doesn't come up again after that, close it (or let the stale bot).

Thanks!

[github-actions]

This issue has been marked stale because it is tagged awaiting-info for 20 days with no activity. Remove the stale label or comment to prevent the issue being closed in 10 days.

[osaxma]

Hi @DanTup

I'm not sure if this is the same issue (in line 12, I believe the checkmark should be next to main)

To reproduce:

• git clone https://github.com/isoos/postgresql-dart.git
• open tests/logical_replication_test.dart file
• Run the test (either using git lense or from the testing tab)
  • the test needs Docker to be installed and may download a postgres image. Though I think a failing check mark would be placed at line 12 for timeout if docker not available.
• You'll see a spinning indicator in line 12

Sorry wrote this in a rush before I forget about it...

---

• Dart SDK version: 2.19.0-401.0.dev (dev) (Mon Nov 14 23:59:52 2022 -0800) on "macos_x64"
• Extension version: v3.52.1
• VS Code version: 1.73.1

[DanTup]

I don't know if it's the same issue either, but since one issue was fixed by VS Code which may or may not have been the original issue, we can use this issue for the above (since I can repro and it should be easier to track down), and if any other issues continues after this is fixed, we can file a new issue.

I can repro your issue, but small refactors to try and get it into a small single file seem to stop it from happening. I'll do some debugging and see what's going on. Sorry for the delay!

[DanTup]

So, I expect this probably is the original issue you saw. What seems to be happening is that then setUpAll and tearDownAll calls inside usePostgresDocker are being registered as tests (which is somewhat expected - it's how success/failure is shown for those calls), but assumed to be in the test file:

The JSON from package:test looks reasonable.. it looks as if we're using the correct line/col (A+B) but the wrong file (D instead of C):

{
	"seq": 12,
	"type": "event",
	"body": {
		"test": {
			"id": 3,
			"name": "(setUpAll)",
			"suiteID": 0,
			"groupIDs": [
				2
			],
			"metadata": {
				"skip": false,
				"skipReason": null
			},
			"line": 10, // A
			"column": 3, // B
			"url": "file:///Users/danny/Dev/TestStuff/postgresql-dart/test/docker.dart", // C
			"root_line": 8,
			"root_column": 3,
			"root_url": "file:///Users/danny/Dev/TestStuff/postgresql-dart/test/logical_replication_test.dart" // D
		},
		"type": "testStart",
		"time": 570
	},
	"event": "dart.testNotification"
}

[DanTup]

(sorry for the spam, I tend to keep notes on issues in case I get distracted and have to come back to them 😄)

It seems we use root_ for both cases here. We're actually using line 8, but there's an off-by-one somewhere. So the ticks should actually show up against usePostgresDocker.

It's a little odd/confusing having them show up this way, but it's how package:test presents them and if we don't show them, it wouldn't be so obvious when a failure is caused by a setup/teardown:

So I think if we fix the off-by-one error, maybe that would be fine.

However - given that, I'm now less convinced that it's the same issue as the screenshot right at the top 😄

I'm outta time for today, but will do some more digging into this tomorrow.. I'm hoping to do an extension release tomorrow, but if this is an easy fix I'd like to get it in the release.

[DanTup]

I've pushed a fix for the off-by-one error:

It's a little odd (confusing?) that this appears here, but the general design here is that if your test calls functions in other files (which may call test or setup/teardown) we use the frame from the stack that is in the file where the entry to the test is. This avoids all of your tests being attributed to some shared "runTest" function if you have some wrapper around the test function.

I think it would probably be better if VS Code let us mark setup/teardown as "not runnable", so we can still show them in the tree (to indicate where the failure was), but without them behaving like tests that can be run. Right now if you try to run one of these, it will fail.

I'll file a VS Code issue and see what the response is. If they don't want to do this, maybe we can reconsider showing them in the tree in this way.

If you see any additional issues with bad alignment like this, let's file new issues to investigate them specifically. Thanks!

[DanTup]

Looks like we might actually be able to already restrict this, so I've filed #4286 about this.

[osaxma]

Thank you @DanTup for your time and efforts -- they are much appreciated 🙏🏽

[DanTup]

np! Thanks for taking time to file issues and provide repros, it makes things much easier for me to track down and fix :-)

[louis-grimaldi]

Any updates ? Still happening very frequently.

[DanTup]

@louis-grimaldi this issue has been closed for a long time - if you're having trouble please open a new issue and include as much detail as you can. Thanks!