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

Provide filter logic of TestNodes; starting with filtering by outcome #1155

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

rowo360
Copy link
Contributor

@rowo360 rowo360 commented Oct 24, 2024

This PR is the first incremental step for issue #1148.
It provides the basic filter functionality of test nodes by supporting filtering by outcome. There's only one single use case supported yet: test results are already present from a test run and afterwards an outcome filter is applied programmatically. Then the test tree will display only the filtered nodes.

Further use cases will be supported in the next incremental steps:

  • UI to configure a filter
  • Apply active filter already during test run
  • Apply commands on filtered nodes only (for example 'Run selected')

So, it's not the intention that this filter functionality is already perfect. But I believe it's a good foundation for the next steps.
These changes do not yet have any effect on the user as the filters cannot be activated, so this PR could be merged without causing any side effects.

If things go as planned, the new filter class can be extended to support also duration and category filtering - and maybe name filtering sometime in the future. And also the other way around: I hope that we won't have to adapt any other code locations if we extend the filters. This applies especially for the test tree: displaying a filtered tree and the filter functionality is well separated.

I tested the filter functionality by misusing one of the existing toolbar buttons. So for example setting the filter programmatically to:
_model.TestCentricTestFilter.OutcomeFilter = new List<string>() { "Not Run", "Failed" };

@CharliePoole
Copy link
Contributor

I'm a TDD guy, of course, so I'd like to see a few tests even if you are still experimenting. However I'm OK if you want to wait. If you add tests, I think we would only need one for the main presenter of the type "wnen the user changes the filter in the view the model is notified" and one for the tree view presenter "when the model changes the view is changed."

Copy link
Contributor

@CharliePoole CharliePoole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great for the purpose. My comments are all minor, so I'll merge and you look a them on the next time you pass.

I realize IsVisible has to be an attribute of the test node because we don't even create a testnode if it's false. It's real name should be something strange like "ShouldDisplayInTree" and only applies to the NUnitTree strategy at the moment. In future we might want to eliminate it as a property and make it a method of the tree display strategy or of the presenter itself, I'm not sure.

@@ -76,6 +77,10 @@ public TestFilter GetTestFilter()
public bool IsAssembly => Type == "Assembly";
public bool IsProject => Type == "Project";

/// <summary>
/// Controls if the TestNode should be visible or hidden in the TestTree
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"In the NUnit Test Tree" right?

@CharliePoole CharliePoole merged commit 54a7abd into main Oct 25, 2024
2 checks passed
@CharliePoole CharliePoole deleted the issue-1148 branch October 25, 2024 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants