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

WIP Enable filtering of killing test units #15

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dmitry-timofeev
Copy link
Contributor

Enable filtering of tests in a test unit once a killing test has
been discovered and the ResultCollector requested the test unit
to stop execution.

This might be beneficial when JUnit5TestUnit represents
a test container (e.g., @RepeatedTest, @ParameterizedTest or
@TestFactory) that has a large number of tests or
some expensive tests.

See also:

@SuppressWarnings("unused") // Used through reflection by the ServiceLoader
// when this extension is requested by JUnit Jupiter Engine
public ConditionalTestFilter() {
this(TestUnitExecutionsRegistry.getInstance());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not entirely happy with this global registry, but haven't found another way to inject an object into the extension (each created launcher instantiates an engine which, in turn, creates a new extension — one cannot supply an extension through launcher, because it is not aware of engine-specific extensions).

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a better solution, but I'd be very reluctant to merge in anything that introduced global state.

Just had a scan of the junit 5 apis and the "smaller test unit" approach might be easier for junit 5 - assuming that parameterized tests etc are identifiable with their own ids during the scan stage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

assuming that parameterized tests etc are identifiable with their own ids during the scan stage.

I guess that they can only be identified if they are executed, because the framework cannot possibly determine how many items a method source will return (or how many dynamic tests a @TestFactory will generate). Another questions to consider:

  1. If we run parameterized/dynamic tests one-by-one, won't that cause too much overhead? In this case, for example, for N executions you will have to call the source method N times (and its implementation will likely instantiate a whole collection of parameters), whereas when you tell JUnit to execute a test container, it will call the source method only once.
  2. If the source method/test factory are not deterministic, I'm not sure we can reliably identify them.

Copy link
Contributor

Choose a reason for hiding this comment

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

The only way to know if the performance impact is significant would be to run some experiments, but as I think you are probably correct that the individual ids will not be discoverable at the scan stage the point is moot.

Which takes us back to trying to break out of the running test again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some benchmarks of the various options are essential indeed.

There is a related discussion regarding APIs to terminate dynamic (a subset of test containers) tests early, where I tried to summarize the issue with the approach in the present prototype: junit-team/junit5#431

… in a class with several tests
Enable filtering of tests in a test unit once a killing test has
been discovered and the ResultCollector requested the test unit
to stop execution.

This might be beneficial when JUnit5TestUnit represents
a test container (e.g., @RepeatedTest, @ParameterizedTest or
@testfactory) that has a large number of tests or
some expensive tests.
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