Skip to content
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

[Test UI] Feature Request: Different Organization Options #126938

Closed
JustinGrote opened this issue Jun 22, 2021 · 14 comments · Fixed by #130731
Closed

[Test UI] Feature Request: Different Organization Options #126938

JustinGrote opened this issue Jun 22, 2021 · 14 comments · Fixed by #130731
Assignees
Labels
author-verification-requested Issues potentially verifiable by issue author feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders testing Built-in testing support verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@JustinGrote
Copy link
Contributor

JustinGrote commented Jun 22, 2021

@connor4312
Related to: #126932

There may be different ways a user wants to view their tests, such as sorted by tag/function or sorted by unit/integration/acceptance/etc. These views may require that a test be put in multiple locations in a tree, since a test may have one or more tags for instance.

This seems it would require either:

  1. Allow a test to have multiple parents (suites could probably still be restricted to only one parent)
  2. Have an additional member that defines "secondary parents" for a visibility perspective.

And also allow this dropdown to be hooked into to provide custom test trees (e.g. "Group by Tag", "Group by Function", "Group by Type". If there are multiple test providers where their root objects are shown, maybe put the "group by" as a dropdown at that level instead

Ideally the extension can register and supply whatever additional trees it wants, maybe via a new root object but would need to reference all the unique IDs already present in the main root, or otherwise some sort of "view" class

image

This goes into the "nice to have" and "vNext" category, it's not a must for initial release.

@connor4312
Copy link
Member

I think @matepek also requested test tags. Is a generic tags: string[] on test items enough to do everything you need here? Or maybe Record<string, string> to group by a key? How would it work if tests are given from multiple providers that might have different sets of tags or organization?

@connor4312 connor4312 added feature-request Request for new features or functionality testing Built-in testing support labels Jun 22, 2021
@connor4312 connor4312 added this to the Backlog milestone Jun 22, 2021
@JustinGrote
Copy link
Contributor Author

JustinGrote commented Jun 22, 2021

I don't consider myself smart enough to propose the ideal API implementation, but what I know needs to work:

  1. When things are organized by a tag, if the tag is clicked to "run/debug", I would expect the runTests to be called with all the tests associated with that tag.
  2. If a test shows up under two different tags (e.g. "Unit" and "MyApp"), when the test is run it should update in both places in the UI. As long as it's still referenced by its unique ID that shouldn't be a problem.
  3. I would assume the root header would still preserve, so if you have "Pester" and "Markdown" test providers, they would each have their own root headers still and the tags would be organized under each respectively. If only one provider is loaded then the root header is omitted as is the behavior today.

tags: string[] would be simple to implement but I feel like the test provider should still provide the "view" so as not to pigeon-hole organizing tests into whatever vscode decides to support (in the same way the current parent/addChild method allows extensions to build the test tree to whatever makes sense to their provider). I'm not sure the best way to do this, maybe a view that allows for a parent/child hierarchy of Record<string,string> to be supplied, or a function that accepts a root object organized a different way without requiring a complete "rediscovery".

@connor4312
Copy link
Member

For this I've experimented with converting the test filter for a more Marketplace-like box with @ tags and autocomplete. I also moved the current directives to "show tests in current document" for instance to be @ -directives since I didn't want to mix text and non-text search. Likewise I wanted tags to be text-based because the alternative experience would be click on filter -> click on tags -> quickpick for tags you want, which is pretty deep. History is still supported:

https://memes.peet.io/img/21-08-f9d0cc7b-42c5-4fcc-a4b4-e1eb201ed7d3.mp4

@connor4312
Copy link
Member

@jdneo I saw that you have an intended use for these too -- does the UI I showed in the video work for you, any suggestions? 🙂

@jdneo
Copy link
Member

jdneo commented Aug 14, 2021

Thank you @connor4312 ! The video looks promising.

There is another scenario came up in my mind: In a lazy-load test explorer with some children haven't been resolved yet. Will users want to search all the items related to a tag?

I'm not sure if this will be a request from users in the future though.

@connor4312
Copy link
Member

connor4312 commented Aug 16, 2021

Yea, when dealing with tags it only filters to tags and items that we know about. We could trigger a global expansion of every node but that could result in a performance cost. Thoughts?

@JustinGrote
Copy link
Contributor Author

JustinGrote commented Aug 16, 2021

@connor4312 sounds like something that should be a configurable option :). "Search undiscovered Tests" or something like that, should default to false.

In addition, maybe when searching have an additional dummy entry or some sort of clickable link that says "more..." or "search all test files" or something so that you can have both.
image

@connor4312
Copy link
Member

Yea we could have some additional text at the end of the tree saying something like "There may be more undiscovered tests matching this filter, would you like to [discover all tests]?"

@connor4312 connor4312 modified the milestones: Backlog, September 2021 Aug 16, 2021
@jdneo
Copy link
Member

jdneo commented Aug 17, 2021

we could have some additional text at the end of the tree saying something like "There may be more undiscovered tests matching this filter, would you like to [discover all tests]?"

Sounds good to me!

@rzhao271 rzhao271 modified the milestones: September 2021, August 2021 Aug 27, 2021
@rzhao271 rzhao271 added verification-needed Verification of issue is requested author-verification-requested Issues potentially verifiable by issue author verification-steps-needed Steps to verify are needed for verification labels Aug 27, 2021
@connor4312
Copy link
Member

Verification:

  1. Have or create a test extension that uses test tags. You can use the sample: https://github.com/microsoft/vscode-extension-samples/tree/main/test-provider-sample
  2. Verify you can filter them in the Test Explorer view using the @controller:tag syntax, and get appropriate completion for them

@connor4312 connor4312 removed the verification-steps-needed Steps to verify are needed for verification label Aug 31, 2021
@rzhao271 rzhao271 added the verified Verification succeeded label Aug 31, 2021
@JustinGrote
Copy link
Contributor Author

@connor4312 I don't see tags in the latest sample or in the latest proposed/stable API as far as I can tell. Is it present on testItem or am I missing something?

@connor4312
Copy link
Member

It's in the latest proposed API right now. It should be stable at the end of this iteration.

@JustinGrote
Copy link
Contributor Author

@connor4312 I had removed useproposedAPIs so I just noticed my vscode.proposed.d.ts wasn't actually updating, my bad!

@github-actions github-actions bot locked and limited conversation to collaborators Oct 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
author-verification-requested Issues potentially verifiable by issue author feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders testing Built-in testing support verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@connor4312 @jdneo @rzhao271 @JustinGrote and others