You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Gutter test icons sometimes show on wrong line when helper functions call setup/teardown #4163
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.
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.
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.
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.
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.
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).
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.
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"
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!
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):
(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.
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!
changed the title [-]Misalignment in Run Test Buttons[/-][+]Gutter test icons sometimes show on wrong line when helper functions call setup/teardown[/+]on Nov 29, 2022
@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!
Activity
DanTup commentedon Sep 20, 2022
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 commentedon Sep 20, 2022
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 commentedon Sep 20, 2022
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 commentedon Sep 22, 2022
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 commentedon Sep 22, 2022
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"totrue(thanks again for that awesome tip).I'll circle back if it appears again.
DanTup commentedon Sep 22, 2022
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 commentedon Oct 24, 2022
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.
2 remaining items
osaxma commentedon Nov 16, 2022
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.gittests/logical_replication_test.dartfileSorry wrote this in a rush before I forget about it...
DanTup commentedon Nov 28, 2022
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 commentedon Nov 28, 2022
So, I expect this probably is the original issue you saw. What seems to be happening is that then
setUpAllandtearDownAllcalls insideusePostgresDockerare 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:testlooks reasonable.. it looks as if we're using the correct line/col (A+B) but the wrong file (D instead of C):DanTup commentedon Nov 28, 2022
(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 againstusePostgresDocker.It's a little odd/confusing having them show up this way, but it's how
package:testpresents 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 commentedon Nov 29, 2022
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
testorsetup/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 thetestfunction.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!
[-]Misalignment in Run Test Buttons[/-][+]Gutter test icons sometimes show on wrong line when helper functions call setup/teardown[/+]DanTup commentedon Nov 29, 2022
Looks like we might actually be able to already restrict this, so I've filed #4286 about this.
osaxma commentedon Nov 29, 2022
Thank you @DanTup for your time and efforts -- they are much appreciated 🙏🏽
DanTup commentedon Nov 29, 2022
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 commentedon Jul 31, 2024
Any updates ? Still happening very frequently.
DanTup commentedon Jul 31, 2024
@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!
Fix off-by-one error in test locations for methods that call setup/te…