-
-
Notifications
You must be signed in to change notification settings - Fork 282
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add test if not yet present in dictionary. #2000
base: main
Are you sure you want to change the base?
Conversation
If I remember right, that condition is adding tests to the explorer when we've previously been able to match a non-standard location (e.g. a referenced, but not directly nested testList in Expecto). The displaced fragment cache is constructed beforehand and isn't being modified in I ran some testing and something is definitely off about the explore behavior on code update. Renaming methods is causing them to disappear. I am able to add and remove test methods and see live updates though. |
Similarly, when I set The reason I want |
If I'm understanding right, you want to discover tests on load via code analysis alone and not running tests. That works against the base assumptions of the test explorer. A lot of the work I did was to re-orient the test explorer around test results since the code analysis results were causing flakiness (i.e. incorrectly located tests which broke the explorer). Tests can still be discovered when AutoDiscoverTestsOnLoad is false by refreshing the explorer manually, which will run tests. |
Yes, this used to work fine for me when I originally ported the test explorer code from Neptune. The notifications work fine for me, this should also be possible to work with. |
My concern is that building from code analysis results would re-introduce the issues outlined in #1874. Test results aren't always the fastest, but they are a consistently reliable method for constructing the test explorer. I'm open to suggestions. |
I would go for some middle ground here. My suggestion would be to always process the incoming notifications. This seems to be off when |
Building the test explorer from code analysis isn't a matter of leaving it on when result-based updates are off, it'd require a separate mode. Currently, code analysis is not used as a primary truth source. It relies on the baseline provided by test results to prevent the unusable states described in #1874 (i.e. where run all doesn't work anymore). Though, the explorer now should be able to run when non-existent tests are selected and be refreshed from test results if it gets off. Perhaps a "trust all code analysis" mode wouldn't get the explorer into an unrecoverable state. |
I've been digging a bit to understand how Visual Studio manages test location discovery. Maybe that's the way forward here too? Then we're capable of anything VS is capable of and we can offload discovery to the adapters and hopefully some of the orchestration to vstest. I'll keep digging. |
Thanks @farlee2121! I appreciate the collaboration here! |
I've not gotten as much research into this as I'd hoped, but I did poke around VS Test and found that this is the package meant to smooth integration for editors: Microsot.TestPlatform.TranslationLayer. |
@farlee2121 I could be wrong but this seemed to help to show the new tests in my project.
Reasoning: the newly added test won't be in the existing dictionary and the check needs to be negated.
I could be wrong, please let me know if this makes sense.
Fixes #1997