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

Apply test filter for test run #1172

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Apply test filter for test run #1172

wants to merge 1 commit into from

Conversation

rowo360
Copy link
Contributor

@rowo360 rowo360 commented Jan 13, 2025

This PR is another contribution to the test filter functionality (#1148, #1147 and #1161).
It contains my proposal to apply the active test filter to a test run. So far all test filter contributions affects only the visual TreeView by showing/hiding tree nodes. But a test run didn't consider the test filter so far.

In general a test run should consider the active test filter. For example, if a test filter only displays a single test, then only this single test should be executed, even if you start 'Run all tests'.

I tested this proposal successfully using these different test run types:

  • run all tests
  • run selected tests
  • rerun tests
  • run failed tests

Unfortunately this PR contains a dozen of changed files. But actually I was really happy that I managed to solve it by one central adaptation. Although the result of changed files might indicate that it wasn't that central...
I hope that it's still possible to review the changes.

The general idea of this proposal is to use a TestId-Filter which contains all IDs of the visible TestNodes. The IsVisible property of a TestNode is set by the filtering - so the set of visible TestNodes represents exactly the result of the filtering.
I'll comment on the indivdual changes in the next comments.

@rowo360 rowo360 self-assigned this Jan 13, 2025
@rowo360
Copy link
Contributor Author

rowo360 commented Jan 13, 2025

All kind of test runs finally execute the method RunTests(TestRunSpecification runSpec) in class TestModel. So I was able to add a central change here to apply the test filter for all kind of test runs.

This central change first checks if the NUnit DisplayFormat is used and if any visual TestFilter is active. Only in that case we call a new method TestFilter.MakeVisibleIdFilter(runSpec.SelectedTests) to apply the filter.
The runSpec.SelectedTests contains the TestNodes matching to the use case - Run all tests, Run failed tests, Run selected tests or Rerun tests.

@rowo360
Copy link
Contributor Author

rowo360 commented Jan 13, 2025

I needed to extend the filter implementation to determine if any filter is currently set. Therefore I added a new property IsActive to the filter functionality - it's implemented by all the invidual filters (TextFilter, OutcomeFilter, CategoryFilter). So several changed files are due to this IsActive property.
This property is required to check if any visual filter is currently set. If not, I don't want to create a new filter for the test run.

@rowo360
Copy link
Contributor Author

rowo360 commented Jan 13, 2025

The actual filter is created in class TestFilter. It iterates recursively through all TestNodes and builds up a TestId filter. As soon as a TestNode is processed which is not visible, the processing is stopped for that part.
I decided to keep the approach simple by adding only IDs from leaf nodes (the test case nodes). So the resulting TestId filter only contains IDs from test cases and not not IDs from TestFixtures or TestSuites.
That could be changed of course. A different approach could check if all children of a TestNode are Visible and add only the ID of the parent in this case. The resulting TestId filter would be shorter in this case - but I don't know if this is important.

@rowo360
Copy link
Contributor Author

rowo360 commented Jan 13, 2025

I guess that the changes in class TestCentricPresenter needs some explanation:
Unfortunately the 'Run failed tests' functionality didn't work with the previous changes. And I learned that in this use case ResultNodes (instead of TestNodes) are passed to the model.RunTests(...) method. ResultNodes are of course specialized TestNodes and thus contain also a property IsVisible.
However when applying a test filter, I set the IsVisible property only in the set of testModel.LoadedTests, but all the ResultNodes remain untouched. Which means that the IsVisible property is never set for a ResultNode.

I solved this issue now, by retrieving all the corresponding TestNodes for the ResultNodes and passing the TestNodes to the RunTests(...) method. This solution works fine.
But I need some guidance about the IsVisible property at a ResultNode: should we keep it untouched or should we apply the test filter also to the set of ResultNodes?

@rowo360
Copy link
Contributor Author

rowo360 commented Jan 13, 2025

I have the impression that it's possible to refactor some part in class TestModel
The class contains a property CategoryFilter and also SelectedCategories. If I understand it correctly, these properties are no longer set at all and I believe could be removed. I assume that they are still left over from the old category filters.
Should I create a 'refactor' issue for this one?
(After we finished the new filter implementation and really succeeded :-))

@rowo360
Copy link
Contributor Author

rowo360 commented Jan 13, 2025

I added several unit tests for the new implementation. In this process I changed some of the filter classes from internal to public - just to access them from the TestCentric.Gui.Model.Tests project.
Would it be OK to use the InternalsVisibleTo attribute for this purpose?
(In a separate issue)

@rowo360 rowo360 requested a review from CharliePoole January 18, 2025 09:31
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.

1 participant