Support test tags

[connor4312]

Thanks for the feedback on runnability. I'm considering something like this, but will continue to think it over:

One solution to this that I think could work decently well is having a run configuration be able to define one (or more) "tag" strings, and allow tests items to have an array of tag strings. For any configuration where tags are defined, the editor would only apply that to test items that have the same tag. We had a request for tagging before as an organizational tool, but that could also work for this.

So for example you could have a custom "runnable" tag not present on subtests, and the presented Go scenario could have a "runnable-darwin" tag to indicate a test only runnable on a certain platform/under certain configurations.

interface TestRunProfile {
  tag?: TestTag;
  // ...
}

interface TestItem {
  tags?: readonly TestTag[];
  // ...
}

class TestTag {
  readonly id: string;
  // maybe more things like a label later, if this is ever exposed in UI
}

Originally posted by @connor4312 in #122208 (comment)

[connectdotz]

hmmm... I am wondering why introducing yet another identifier (tag), would it be simpler and more flexible by just asking the TestItems if they support the profile? for example

interface TestItem {
  canRun?: (runProfile: TestRunProfile): boolean;
  // ...
}

[connor4312]

We need to know that information statically. Or we would have to call that method for every test item as soon as it's created and assume it never changes for its lifetime. Using tags to correlate them in a declarative way removes that assumption.

[connectdotz]

We need to know that information statically.

But it is not static though... if the profile changed, wouldn't the test item tag might change as well? Also if the user changed their settings like disable jest watch mode, we do want the test items to be able to run again, i.e. change the tags?

[connor4312]

if the profile changed, wouldn't the test item tag might change as well?

That's fine, but it's more that we can't call a method on demand. For example, whenever you hover over an item in the test tree, we need to know whether to show the run/debug button. We always need to know when to show the "run all" and "debug all" buttons. I think "declarative" is a better word than "static"

Also if the user changed their settings like disable jest watch mode, we do want the test items to be able to run again, i.e. change the tags?

Yea, you'd be able to change the tag either on every test item, or on the RunProfile itself. For instance if you wanted to disable manual running while the watch was running, you could have a "Disabled" tag that you set on the profiles and don't assign to any tests.

[connectdotz]

I think I see your points.

And looks like you don't want to use "id" of the profile either? So basically tags, like kind, is just another way to "group" profiles for TestItem... hmmm... maybe we don't need 2 grouping mechanisms? "Kinds" can be just pre-defined system tags? This will also benefit use cases, like ours, who can operate on the "kind" level of granularity without creating/managing redundant tags...

[connor4312]

I see your point. The tag direction came out of the Go tags that @firelizzard18 mentioned, where tests are tagged to run (or not) on certain platforms or on different build tags. The functional change is that, in the tag approach, adding a new run profile that applies to all tests is the default behavior and requires no mutation, where if IDs were listed individually it would require every test to be mutate. Tags were also requested as an organizational feature by @JustinGrote in #126938

[firelizzard18]

@connor4312, the tags you describe feel like the tags mechanism for GitLab CI more than Go tags, which is a good thing IMO.

Go tags

In effect, Go tags are a Boolean Algebra. Each tag is mapped to a boolean variable, and answering "should I compile this file" requires evaluating a boolean expression. Each tag that is defined, either implicitly by GOOS/GOARCH or explicitly by -tags foo,bar, is a boolean variable with a value of true. Every other tag is implicitly a boolean variable with a value of false. Determining whether to include a file in a build comes down to evaluating a boolean algebraic expression.

For example, GOOS=darwin GOARCH=amd64 go build -tags gtk will compile a file that declares //go:build darwin && gtk, but not a file that declares //go:build !gtk, //go:build amd64 && cocoa, etc.

IMO, a boolean expression/algebra solution is an unnecessarily complicated solution for this issue.

GitLab tags

GitLab tags can be considered a set-based system. Each job defines a set of tags. Each runner defines a set of tags. A job may be dispatched to a runner if and only if the runner's tag set is a strict superset of the job's tag set. In plain English, a job may be dispatched to a runner if every tag in the job's tags is also in the runner's tags.

For example, a runner with tags linux,docker can run a job with tags linux or linux,docker but not a job with tags windows or linux,kubernetes. However, there is no way to make a job that explicitly won't run on a docker runner (as in !docker in Go).

VSCode test tags

IMO, Go's tags are an unnecessarily complicated for this problem. There are three versions of tags sets that make sense to me:

1. A profile has one tag, a test has many tags - I read this as, "Each test defines the profiles it can be run by"
2. A test has one tag, a profile has many tags - I read this as, "Each profile defines the tests that it can run"
3. Profiles and tests have many tags (like GitLab)

The behavior and 'meaning' of (3) depends on what operation is used to 'match' a profile and a test: "profile.tags is a superset of test.tags" or vice versa, or "profile.tags and test.tags have at least one tag in common". In either case, (1) or (2) could be changed into (3) at a later date by changing tag: TestTag to tag: TestTag | TestTag[], so it's probably better go with (1) or (2) for now.

I have a slight preference for (2) over (1), because I prefer to think in terms of "this profile can run " instead of "this test can be run by ". But that would not work for organizational tags.

Tags vs kind

@connectdotz Tags and TestRunProfileKind are similar, but kinds are 'special' tags that serve an additional purpose: they indicate whether running with that profile will start a debugging session, provide coverage information, or just run the tests. Also, they are mutually exclusive at the moment. Whereas tags would just be a tool to match tests with run profiles. IMO it would be a mistake to merge kinds and tags, because that would necessitate special cases to handle those predefined system tags. Whereas with kinds and tags separate, handling tags is straightforward and has no special cases.

[connectdotz]

interesting discussion...

Tags were also requested as an organizational feature

I wonder now with the ability to have multiple TestController, would that be an alternative way to do this? If I have integration vs unit tests, I probably would prefer 2 different "trees" rather than mingled? Bonus: RunProfile already scoped under TestController, therefore, no need for an extra mechanism to sort/association/matching/tagging... I am not sure if the multiple-TestController scheme covers the organizational feature requested, just some quick thoughts...

Tags and TestRunProfileKind are similar, but kinds are 'special' tags that serve an additional purpose: they indicate whether running with that profile will start a debugging session, provide coverage information, or just run the tests. Also, they are mutually exclusive at the moment.

The more I think about it, the more I think "Kind" should not be "special"... they just indicate a characteristic of the RunProfile itself, the interpretation of these profiles lies in the extension anyway, which can interpret them just like any other tags. The test explorer, like the extension, is just a tag consumer that happens to use the 'run' and 'debug' system tags for its UI. No new concept is needed.

How about "mutual exclusiveness"? I am not sure if it is applicable for every extension: In our world, "Coverage" is not a stand-alone "thing", it has to go with either run or debug thus a profile should be able to have ["run", "coverage"] or ["debug", "coverage"]. You can argue a profile still should not have ['run', 'debug']. It is probably correct under the current implementation that UI doesn't send over the separate debug-or-run info (otherwise, I would be using exactly the same profile for both). So the question is how do you prevent developers from applying the wrong tag or interpreting it wrong? I think the risk for that applies to all tags... run/debug is not "special" in this regard either...

IMHO, consolidating these 2 similar concepts can simplify the system and hopefully reduce some mental overhead. I know some arguments could be subjective, just want to throw it out there as an alternative viewpoint for @connor4312 to evaluate...

[JustinGrote]

@connectdotz it would totally be doable with different providers, with the fact that they may be inefficient, but this would push the settings into the extension space, it'd be nice to have it as part of the dropdown for a "group by" view effectively.

[connor4312]

The more I think about it, the more I think "Kind" should not be "special"... they just indicate a characteristic of the RunProfile itself, the interpretation of these profiles lies in the extension anyway, which can interpret them just like any other tags. The test explorer, like the extension, is just a tag consumer that happens to use the 'run' and 'debug' system tags for its UI. No new concept is needed... How about "mutual exclusiveness"? I am not sure if it is applicable for every extension: In our world, "Coverage" is not a stand-alone "thing", it has to go with either run or debug thus a profile should be able to have ["run", "coverage"] or ["debug", "coverage"].

I don't think I like this approach as much. In either run, debug, or coverage, users will usually do exactly one of those things when executing tests. I debug when I need to diagnose a failing test, and am usually not interested in coverage at the same time; in some cases debugging with coverage not possible, e.g. nyc-instrumented tests. While "coverage" is technically a special type of "running" it's still a separate action that most users are not taking as their default action all the time. And as you pointed out having both "run" and "debug" together doesn't make sense.

Having these be disjoint actions also makes our UI clear and predictable; there is one button to do each type of action. I'm not sure what this would look like if there were overlaps.

We could still combine them by making TestRunProfileKind a class that has a category, maybe a custom id. However I think I'd prefer to keep these concepts separate, which could be useful if we ever go with approach (3) that Firelizzard mentioned above; we could easily allow profiles with multiple tags, but would never allow profiles with multiple kinds.

[connor4312]

Test tags will be available in the next Insiders. The API includes a label, though this is not yet exposed in the UI. Try it out and let me know what you think.

[jdneo]

Is it possible to make the test item tag filterable in the search bar?

Because in Java JUnit 5, there is a concept call tag, will is used to filtering tests.

[connor4312]

Yep, that's the plan, just didn't quite get there today 🙂

[jdneo]

That's great @connor4312!

[firelizzard18]

@connor4312 I'm having trouble getting tags to work:

const runnable = new TestTag('runnable');
ctrl.createRunProfile('go test', TestRunProfileKind.Run, (rq, tok) => this.run(rq, tok), true, runnable);

const testItem = ctrl.createTestItem(id, label, uri);
testItem.tags = [runnable];

Yet none of the tests show a run button. I'm running with "enableProposedApi": true and:

Version: 1.60.0-insider
Commit: 7c25c174726ab969db3f99058cdf24bcfae1cfbe
Date: 2021-08-11T05:14:12.894Z
Electron: 13.1.8
Chrome: 91.0.4472.164
Node.js: 14.16.0
V8: 9.1.269.39-electron.0
OS: Linux x64 5.10.27-gentoo

I also tried setting TestRunProfile.tag after creating it. If you want to poke around, see this branch, which is a slight modification of golang/vscode-go#1590 (which works).

[connor4312]

Thanks Ethan, I've fixed that as part of #130731

With that PR, tags will now be searchable in the filter box, for more info and a video check out: #126938 (comment)

[firelizzard18]

@connor4312 This works as I would expect, thanks!

[ashgti]

Can you negate a tag filter? E.g. can I do !@tag? Should I file a feature request for the ability to negate a tag?

[connor4312]

Sure, please file a feature request

[connor4312]

Verifier: scan through the API and make sure docs make sense. There were no functional changes in this issue since last verification.