Allow test suites to report errors like tests

[rossknudsen]

Currently addressing an issue with the Jest extension that I've been working on. Sometimes there are issues parsing the test files - so I know what suites and tests are present. I'd like to be able to show errors for suites as well as tests so I can advise the user of what went wrong.

The *Info types are as follows:

export interface TestSuiteInfo {
  type: 'suite';
  id: string;
  label: string;
  description?: string;
  tooltip?: string;
  file?: string;
  line?: number;
  children: (TestSuiteInfo | TestInfo)[];
}

export interface TestInfo {
  type: 'test';
  id: string;
  label: string;
  description?: string;
  tooltip?: string;
  file?: string;
  line?: number;
  skipped?: boolean;
}

In both cases I can display the label, description and tooltip for both suites and tests. So there is no way to indicate that there is a problem to the user (apart from using TestLoadFinishedEvent.errorMessage). It would be cool to have some way to show tests and suites that have issues at this stage.

The *Event types are as follows:

export interface TestSuiteEvent {
  type: 'suite';
  suite: string | TestSuiteInfo;
  state: 'running' | 'completed';
  description?: string;
  tooltip?: string;
  file?: string;
  line?: number;
}

export interface TestEvent {
  type: 'test';
  test: string | TestInfo;
  state: 'running' | 'passed' | 'failed' | 'skipped' | 'errored';
  message?: string;
  decorations?: TestDecoration[];
  description?: string;
  tooltip?: string;
  file?: string;
  line?: number;
}

Here I can display the label, description and tooltips as before for both. However I can for tests, display a message and indicate an error state. It would be cool if we could show error state and a message for suites as well.

I realise that there might be issues with making suites errored when there are passing tests within them as well as many other issues. Feel free to discuss, or point me to previous discussion or advise me on alternatives to solve my original problem. I'm not sure if I can convert a suite to a test and vice versa as errors come and go...

[matepek]

For that I'm creating a dummy TestInfo. It works just fine for me, but if there will be a nicer solution I would like to know about it. subscribed.

[hbenl]

I'm thinking about adding an errored flag to TestInfo and TestSuiteInfo, information about the error could be placed by the adapter in the description and tooltip properties.
Likewise I could add an errored state for TestSuiteEvent, information about the error would still have to be placed in the description and tooltip properties.
@rossknudsen @matepek Do you think that would cover all your use cases?

[rossknudsen]

How about the message property as well? Then if the suite is errored, we can display a stack trace or other lengthy message.

[hbenl]

Currently, Test Explorer only keeps logs for tests but not for suites (and that's where the value of the message property is written to). I thought that description (shown inline in the tree) and tooltip (shown when the user hovers over a tree item) could be enough but that depends on what you want to report there. So I guess I'll add the message property (and suite logs) as well.

[matepek]

If we are going toward on this route we might should consider deprecating the TestInfo for good and adding all its properties to TestSuiteInfo. Because I don't see any more reason to distinguish between them.

[rossknudsen]

Currently, Test Explorer only keeps logs for tests but not for suites (and that's where the value of the message property is written to). I thought that description (shown inline in the tree) and tooltip (shown when the user hovers over a tree item) could be enough but that depends on what you want to report there. So I guess I'll add the message property (and suite logs) as well.

If adding the message field will make this task significantly more difficult, you could split the task in two. I like the message field because if you have something large to display, like a stacktrace, then the tooltip is the only thing that would be reasonable to use. And then you can't copy and paste any of the text to Google and because the font is small, its hard to read.

If we are going toward on this route we might should consider deprecating the TestInfo for good and adding all its properties to TestSuiteInfo. Because I don't see any more reason to distinguish between them.

I like them separate for a few reasons. Firstly, I think that it provides a nice symmetry:

	Info	Event
TestSuite	TestSuiteInfo	TestSuiteEvent
Test	TestInfo	TestEvent

Secondly, they may have the same properties today, but if there was a reason to diverge in the future, it might become messy.

[kondratyev-nv]

In Python Test Explorer, I create nodes for tests/files that have invalid tests (for example, file parsing issues), and I also fire a test event with errored state and a stacktrace in message for them just after finished event. By clicking on such test full stacktrace is shown in Output panel. Parent nodes are already maked errored as well in this case.

IMO, there is no need to have anything special for such cases. Tooltips are inconvenient for errors and showing errors from all invalid tests by clicking on a suite would look messy in the Output.

[hbenl]

If we are going toward on this route we might should consider deprecating the TestInfo for good and adding all its properties to TestSuiteInfo.

The 2 are getting more similar, but there's still some differences: Tests don't have children and suites generally don't have a state (the errored flag would be a special case but I don't want suites to be able to say e.g. "My state is passed even if my children are failed").

IMO, there is no need to have anything special for such cases.

What you describe is a good workaround, but there shouldn't be a need for adapter authors to come up with workarounds. And since the ability to mark a suite as "errored" keeps coming up, I guess it's time to support it.

showing errors from all invalid tests by clicking on a suite would look messy in the Output.

Absolutely, in fact the main reason why I keep a separate log for each test is that I hate having to scroll through endless messy logs to find the error message I'm interested in. So I'd only show the errors from the suite in the suite's log, but not all the errors from the tests in the suite.

[matepek]

The 2 are getting more similar, but there's still some differences: Tests don't have children and suites generally don't have a state (the errored flag would be a special case but I don't want suites to be able to say e.g. "My state is passed even if my children are failed").

Yes, I thought about it and I see no problem with test having children and suite having state. On the other hand suite having "special" flag sounds meh. :)

First but should be last: Either both could be decorated with the other's properties as optional (for backward compatibility) or a new api version should be released. Let's call the "merged" interface as MergedInfo and MergedEvent. It is easier to talk about it.

Let's imagine what would we get and how could we handle the merge of the two types.

• TestInfo having children: Then it wouldn't be a leaf anymore. MergedInfo would be a leaf if it has no child.
• TestSuite having states and events.
  • They have already have running event. So thats fine.
  • Other events would have the current propagation logic (just mentioning):
    • passed child would override skipped
    • failed child would override the passed and skipped.
    • errored child would override the passed, failed and skipped.
• TestSuite is skipped: All of its children are skipped.
• I would found useful TestInfo having children: I don't find now where I mentioned that I wanna add children to a TestInfo during running. Why? Because of this. This information can be detected during runtime only.
• The way how someone would use the message field in case of TesSuiteInfo (MergedInfo with children) totally the choice of the adapter. And that sounds normal.
• If someone would like to use the TestSuiteInfo to indicate error they would have the way without special children as kondratyev-nv and I currently implemented.
• If someone want to use it in the old way, it would totally work.
• Event the completed event makes a small sense in case the MergedInfo not having a child. We can say that it means skipped for backward compatibility so everything would override its status.

This solution is a generalization of TestSuiteInfo and TestInfo.

export interface MergedInfo { type: 'suite'|'test'; ... }
export interface TestInfo extends MergedInfo { ... }
export interface TestSuiteInfo extends MergedInfo { ... }

Same for MergedEvent...

Shouldn't we looking for "general" solutions instead of "special" and "custom" one? Well I think you see my point already.

[kondratyev-nv]

What you describe is a good workaround, but there shouldn't be a need for adapter authors to come up with workarounds. And since the ability to mark a suite as "errored" keeps coming up, I guess it's time to support it.

Sorry, I did not mean that doing nothing is the way. It would be amazing if tests and suites can be conveniently marked as errored.

However, I believe that suites should not be in errored state by themselves. Suites should always aggregate children states, this behavior is expected and already works. For example, errored suite makes no sense if all children in it can pass. So, IMO view and behavior should be similar to what is now achieved with workarounds. If there would be a more straightforward way to do so - awesome!

[hbenl]

I have just published the new API and Test Explorer, you can now set both tests and states as errored when loading or running.
Tell me if it works for your use case.