TestCentric / testcentric-gui

TestCentric GUI Runner for NUnit
Other
68 stars 31 forks source link

Crash "View as XML..." executed on tree category node #1105

Closed rowo360 closed 1 month ago

rowo360 commented 1 month ago

Describe the bug This bug is a continuation of the work to support the different tree view display formats (#1101). There's a crash when executing "View as XML..." on a tree category node.

To Reproduce Steps to reproduce the behavior:

  1. Open a test project
  2. Switch tree view mode to 'Fixture list'
  3. Switch tree grouping to 'Category'
  4. Open context menu on some category node and select "View as XML..."
  5. Crash

Important This crash is not happening only for category nodes, but instead for all kind of group nodes. For example also for "Outcome" grouping - nodes 'passed' and 'not executed' or "Duration" grouping - nodes 'fast' and 'slow'.

Environment (please complete the following information):

rowo360 commented 1 month ago

This crash is happening because of an incorrect cast - so far only simple TestNodes were processed, but now also a GroupNode is passed to this method, which results into a NullReferenceException:

rowo360 commented 1 month ago

On first sight it might seems possible to fix this single line by adding some statements regarding a GroupNode. But that won't help, as we won't be able to work properly with a GroupNode in the next steps.

And after taking a look at the XML structure, I have doubts as to whether this can ever work. For example those grouping nodes like 'passed', 'not executed' or 'fast', 'slow' are not present at all in this XML, aren't they? This would leave only the test categories that are present in the XML structure. But that's also difficult because a category might be spread over several test fixtures or test cases. So a solution cannot just take one single specific part from the XML, but instead must search all occurences of that category and dynamically create some result XML.

@CharliePoole: That sounds tricky! Therefore I propose to disable the "View as XML..." context menu for category nodes. If it's really required at some point of time, we can spend more effort on defining the expected system behavior.

CharliePoole commented 1 month ago

Try changing the cast to an ITestItem. If that doesn't fix it right away, we may need to expand that interface.

rowo360 commented 1 month ago

Good tip: the interface ITestItem is a common interface of both classes! I tried it out quickly, just to see where it would take us.

When I change the _testNode member in class XMLDisplay to ITestItem it shows up these errors.

So, the idea would be to extend the interface ITestItem with exactly these missing properties/methods. But I'm struggeling how exactly the implementation for example of the ID property should look like for a category?

Here's a screenshot from one complete XML I'm currently using for testing purpose: I marked in green the existing Ids for testcases, test fixtures or assemblies. All of these can be easily found by the implementation _model.GetResultForTest(testNode.Id), because there's a matching ID also in the XML. In contrast, I see the categories or result state (which I marked in red) - there's no ID around which can be used directly. Instead the categories or result state are present in several locations in the XML file, so my impression is that the current implementation cannot handle this out-of-the-box. And I have the current feeling that a new, separate implementation for grouping nodes is necessary - if we really want to support this feature :-)

CharliePoole commented 1 month ago

OK, wait a minute. A group doesn't have a test ID because it's not a test. Also, it doesn't exist anywhere in the XML representation of the test result. That maps to the internal representation of a test, which is pretty much the same as the NUNIT_TREE display format. So you were right to disable the display in the first place. Sorry 'bout that.

ITestItem has in it the only two things a group needs: a name and a filter that allows the arbitrary list of tests it groups to be executed. A neat idea even if I say so myself. :-)

rowo360 commented 1 month ago

I would also have liked it if we had come up with a simple and good idea to display some XML for group nodes. But let's stick with the approach of disabling it for now. I'll prepare a PR.

CharliePoole commented 1 month ago

Good. Bear in mind that the XML dialog was shows XML that is sent as a progress message and also ends up as part of TestResult.xml. Groups don't exist in that way. Essentially, we handle them the same as we would handle the individual tests being checked separately. So if we created an XML representation of a group, it's only purpose would be to give this dialog something to display. :-)

In the UI, I'd eliminate the ability to bring up the dialog when right-clicking on a group. When the dialog is pinned, if the user selects a group node, you'll want to show something on the screen. The ITestItem has a name, so maybe that's enough.