Introduce @CombineTestTemplates support

[danielhodder]

This adds support for combining multiple TestTemplate providers
together, in a product style, so that an implementer can use the
benefits of multiple extensions together.

Issue: #1224

Overview

This is a fix for #1224. I had some free time so I thought I would take a look at how hard it would be. This will almost certainly not work for everyone so I have made it opt-in with a new annotation, @CombineTestTemplates. This will allow those who need this feature to use it, and everything will be normal for others.

Using the Maven test sample as a quick test we get the following test output (happy to hear suggestions for better test names):

I have not yet had a chance to write test cases but I'm gonna have a crack at that later in the week if this seems interesting.

---

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

---

Definition of Done

• There are no TODOs left in the code
• Method preconditions are checked and documented in the method's Javadoc
• Coding conventions (e.g. for logging) have been followed
• Change is covered by automated tests including corner cases, errors, and exception handling
• Public API has Javadoc and @API annotations
• Change is documented in the User Guide and Release Notes
• All continuous integration builds pass

[codecov]

Codecov Report

Merging #2409 (f86a083) into main (97d3322) will decrease coverage by 1.16%.
The diff coverage is 100.00%.

❗ Current head f86a083 differs from pull request most recent head e501035. Consider uploading reports for the commit e501035 to get more accurate results

@@             Coverage Diff              @@
##               main    #2409      +/-   ##
============================================
- Coverage     91.04%   89.88%   -1.17%     
- Complexity     4473     4559      +86     
============================================
  Files           385      397      +12     
  Lines         10846    11269     +423     
  Branches        845      909      +64     
============================================
+ Hits           9875    10129     +254     
- Misses          744      864     +120     
- Partials        227      276      +49     

Impacted Files	Coverage Δ
.../engine/descriptor/TestTemplateTestDescriptor.java	100.00% <100.00%> (ø)
...junit/platform/engine/EngineDiscoveryListener.java	50.00% <0.00%> (-50.00%)	⬇️
...g/junit/platform/engine/discovery/UriSelector.java	66.66% <0.00%> (-25.00%)	⬇️
...unit/platform/engine/discovery/ModuleSelector.java	66.66% <0.00%> (-25.00%)	⬇️
...nit/platform/engine/discovery/PackageSelector.java	66.66% <0.00%> (-25.00%)	⬇️
...it/platform/engine/discovery/UniqueIdSelector.java	75.00% <0.00%> (-25.00%)	⬇️
...t/platform/launcher/LauncherDiscoveryListener.java	75.00% <0.00%> (-25.00%)	⬇️
...atform/engine/discovery/ClasspathRootSelector.java	66.66% <0.00%> (-25.00%)	⬇️
...rm/engine/support/descriptor/DefaultUriSource.java	75.00% <0.00%> (-25.00%)	⬇️
...t/platform/engine/discovery/DirectorySelector.java	71.42% <0.00%> (-21.43%)	⬇️
... and 205 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97d3322...e501035. Read the comment docs.

[sbrannen]

Tentatively slated for 5.8 M1 solely for the purpose of team discussion

[SpencerMelo]

Any idea on when we are going to be able to use it?

[SpencerMelo]

Hi folks, here I'm again, any idea when this will be available?

Best regards!

[parnmatt]

I too am quite interested in if/when this would be accepted.
The assignee and reviewer lists are still empty; so could be a while.

[mikalaikarol]

Looking forward for it as well

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be closed if no further activity occurs. If you intend to work on this pull request, please reopen the PR. Thank you for your contributions.

[stale]

This pull request has been automatically closed due to inactivity. If you are still interested in contributing this, please ensure that it is rebased against the latest branch (usually main), all review comments have been addressed and the build is passing.

[mikalaikarol]

sad that there was no continuation on this PR

[marcphilipp]

Sorry, we haven't managed to discuss this PR in the team, yet. Hence, I reopened the PR.

[rmodini]

@marcphilipp thanks for reopening the pr! looking forward to this

[marcphilipp]

Did any of you have any concrete use cases in mind except for combining repeated and parameterized tests (when is that actually useful?). JUnit Pioneer has @CartesianProductTest. Does that address your use case?

[marcphilipp]

Also related:

• Introduce extension API for container templates #871

[danielhodder]

The original use for this was for testing database migrations if I recall. We have versioned migrations (1.0, 1.2, 2.0, etc.) which are all cumulative. We also support multiple databases. So the idea was run tests after upgrading from each version to the latest, on al the supported databases.

You'd end up with tests like:

• Test the feature works on a freshly created Oracle database
• Test the feature work on a database that started on the 1.0 schema and was upgraded to the latest on MS SQL
  and so on.

The main issue I see with things like the CartesianProductTest and other similar things, is they assume a simple value. In my case I was creating new databases for each test so there's a lot of setup code to get that all going, and I think each execution of the template ends up registering about half a dozen specific ParameterResolvers and Before/AfterCallback hooks.

The basic goal was to have a test helper which let you focus on writing the test code, and not gluing together the test libraries by passing dynamic parameters off to the different test helpers yourself. Basically you go from:

@EnumSource(DatabaseTypes.class)
@VersionDetector
public void test(DatabaseType database, String startingVersion) {
    try (Database d = helper.createDatabase(database)) {
        DataSource source = d.getDataSource()
        // Run some scripts
        scriptHelper.runScriptsFrom(startingVersion, d.getAdminDatasource());

        try (ApplicationPlatform a = platformHelper.initialize(source)) {
            // Now test
            assertTrue(a.foo());
        }
    }
}

To something more like

@ApplicationDatabaseTest
public void test(ApplicationPlatform a) {
    assertTrue(a.foo());
}

And the @ApplicationDatabaseTest annotation is a meta annotation which does all the setup, script running, and application initialization. Then provides the initialized platform with a ParameterResolver. It also deals with cleaning up the test databases, and the global state the ApplicationPlatform needs to work correctly.

Hope that helps give an example of where this kind feature is super helpful. It is possible to do today but you have to write the whole thing from scratch, rather than being able to assemble it out of the existing elements Junit provides.

[marcphilipp]

@danielhodder Thanks for the detailed description! I wonder if we should rather prioritize implementing #871 so that you could run the test class multiple times against different database types and the contained test methods against the different versions. Would that also work in your case?

[danielhodder]

Not so much for us. We don't want to have two tests that use the same database. Basically we want a brand new database set up for each test (isolation etc.). A lot of the applications need to setup things which would collide so it's easier to not.

In fairness though that's partly because that's how our test support has always worked, and so just porting the existing behavior is easier.

Another use case for use, which I thought just today is take the previous example, and then parameterize each test with a known set of Demo Data.

Basically #871 doesn't really change the overall landscape of only being able to template a test with one provider. The goal of this PR is to create a way of having smaller, re-usable, templating methods and then compose them as needed in your test; rather than having to create a custom extension for every test class.

Don't get me wrong though #871 would be awesome and that would definitely help for other use cases we have. (Bonus points if we can have a version of that at the Suite level so you can deploy servers with it.)

[ewoks]

Any updates here? Anything to be resolved before it lands to next release?

[danielhodder]

I've updated the copyright notices which were failing the spotless check.

[marcphilipp]

We've looked at this today in the team call. We're not yet convinced this would be easy to grasp from a user's perspective. Since #871 is less controversial and more generally useful, we think we should implement that first.

[danielhodder]

I don't think I'd argue that feature would be for very advanced usage only, and very much hard to grasp, if you didn't get it. I guess this exists to solve a specific problem which can't currently be handled in extensions. If there was a way of accomplishing this outside of the core engine I'd gladly write it there. As far as I can tell that's not possible though.

This would be a feature mostly of interest to large teams build big software packages which have a lot custom test extensions, which the team may not own. In our case most of these templates are managed by our 'platform' team, while the usages are in downstream app teams. If we can combine templates then teams don't have to choose between them.

I do thing #871 is a great step and might help if it's possible to use that with inner classes. That would let you have something like:

class Test {
    @Parameterized
    class InnterTest { 
         @MyDatabaseTestTempate
         public void test(DataSource db) {
         }
    }
}

It's not as 'clean' but it accomplishes a similar end, although it does mean you have to have separate extensions for these, rather than re-using existing ones, which was my goal.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be closed if no further activity occurs. If you intend to work on this pull request, please reopen the PR. Thank you for your contributions.

[danielhodder]

Just ran into a more simple example which might help demonstrate this one. I am trying to combine the PamaterizedTest annotation from JUnit 5, with the RetryingTest annotation from JUnit Pioneer. In the current implementation if I try and use the two together I won't get parameters on any of the runs which the retrying test template generates.

#871 would definitely help since it would also allow BeforeEach and AfterEach methods to be parameterized.

[slinkydeveloper]

Hi all, I have a similar use case to @danielhodder which can be solved by this PR. I'm trying to figure out how to develop matrix tests where input args are injected partially by ParameterizedTest, and partially by custom ParameterResolver. I don't want to create nested classes as this is not what I need, and in general I might need several ParameterResolver. For example: I have a ParameterResolver for MyObject, I wanna develop a test where I use my TestTemplateInvocationContextProvider which gives me the object X in configuration A, B, C and then I wanna use ParameterizedTest, so at the end the test method is something like:

@ParameterizedTest
@ForAllConfigurationsOfMyObjectTest
void myTest(MyObject x, Integer someValueFromParameterizedTest) {}

Something like this interface perfectly fits my needs, and it's also an elegant understandable solution.

[danielhodder]

I spent a bit of time and wrote a basic version of this (with a lot of hacks) here: https://github.com/danielhodder/junit-test-template-combiner. If there's enough interest I'll figure out how to publish this to maven central.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be closed if no further activity occurs. If you intend to work on this pull request, please reopen the PR. Thank you for your contributions.

[stale]

This pull request has been automatically closed due to inactivity. If you are still interested in contributing this, please ensure that it is rebased against the latest branch (usually main), all review comments have been addressed and the build is passing.