Skip to content

Adds DiscoveryCallbacks #577

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

Closed

Conversation

smoyer64
Copy link
Contributor

As described in #354, this PR provides access to the test plan before it becomes immutable and facilitates the creation of extensions to process parametric tests as well as to guarantee test order. See #354 for a much longer rationale.

Like others, I had shelved this concept until M3 was out but I'd like to participate in the discussion since parametric tests (via JUnitParams) are one of my favorite patterns and I've been waiting for this ability in upREST. This extension point is extremely powerful and therefore also dangerous. I'd be happy to review other ways that parametric, ordered, scenario, theories tests might work and there's definitely value in only having one supported mechanism for each.


I hereby agree to the terms of the JUnit Contributor License Agreement.

@marcphilipp
Copy link
Member

I don't think the rebase worked... You didn't really change 330 files, did you?

@smoyer64 smoyer64 force-pushed the feature/discovery-callbacks branch from 04022d7 to 830f6b3 Compare December 1, 2016 14:34
@smoyer64
Copy link
Contributor Author

smoyer64 commented Dec 1, 2016

No I didn't! I'm not entirely sure how I produced that commit (I'll look later as it's on another laptop).

This PR is really for team discussion and should be labeled "don't-merge" since we don't really want the DiscoveryReportExtension to be added to the default ExtensionRegistry (it's only a demonstration of how the new callback works).

@PeterWippermann
Copy link

I'd be really happy to see this proposal making progress, because I agree with @smoyer64 that this extension point would be very valuable. In my current understand this is the key feature to enable parameterized tests as we had them in JUnit 4.
@marcphilipp could you foster an internal discussion in your team please?

@marcphilipp
Copy link
Member

We are going to meet next week and will discuss this proposal. Thanks for your patience!

@marcphilipp
Copy link
Member

Please note that we've discussed parameterized tests in the team and they will be the main theme for M4. However, since we want to support use cases with a large number of parameters that may be read from a file etc. we will not create a TestDescriptor for each parameterized invocation during discovery. Instead, we will report additional TestDescriptors during execution, similar to how dynamic tests work under the hood. The programming model will probably not use @TestFactory methods, rather something that looks very similar to a regular @Test method. We'll let you know when we have a detailed proposal. You can track progress by subscribing to #14.

@smoyer64
Copy link
Contributor Author

smoyer64 commented Feb 8, 2017

I'm playing with the @TestTemplate and finding it very much to my liking. It is much more appropriate for parameterized tests. As you noted, this doesn't cause the potential explosion of TestDescriptors for large sets of parameters. This callback may still be useful for various types of sorted tests.

@bechte
Copy link
Contributor

bechte commented Sep 7, 2017

I am wondering if it makes sense to have a single extension point at the end of the discovery phase or if it would make more sense to have a callback after each test descriptor being found, initialized and added to the test plan? Would that also help?

@smoyer64
Copy link
Contributor Author

smoyer64 commented Dec 9, 2017

@bechte - Somehow I missed your comment in September. I think that a callback after each TestDescriptor was discovered would cover a the scenarios described in #354. And since the root container is itself a TestDescriptor, you would be passed the entire hierarchy just like the proposed AfterDiscoveryCallback. The difference is that, if I'm reading the code right, you'd get the root TestDescriptor's callback before it's hierarchy of children was established. Otherwise your idea seems much better than my PoC since you wouldn't necessarily need to traverse the test tree yourself!

@marcphilipp
Copy link
Member

We're cleaning out the issue tracker and closing PRs for issues that we've not seen much demand to fix. Feel free to comment with additional justifications if you feel that this one should not have been closed.

@smoyer64
Copy link
Contributor Author

I think TestTemplate is a safer way to go and accomplishes (so far) everything I've tried to do with test permutations.

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.

5 participants