Inheritance support for conditional annotations

[scordio]

I faced a use case where inheritance for @EnabledIfSystemProperty would have helped for less boilerplate.

Specifically, it's a project with many test classes for integration tests that should be executed only on the CI infrastructure and specific runners. Such fine-grained control is currently achieved combining the maven-failsafe-plugin to exclude the local environment and @EnabledIfSystemProperty(named = "it.test", matches = ".+") on each test class to ensure that they are executed only under the appropriate CI jobs (the it.test property is set to different values by each CI job to tune the failsafe plugin execution further).

As the test classes already share common parts in a parent class, pulling @EnabledIfSystemProperty to the parent class would help reduce the boilerplate.

Relates to https://github.com/junit-team/junit5/issues/3462#issuecomment-1721053528.

Deliverables

• [ ] Conditional annotations support inheritance

[marcphilipp]

Thinking out loud here, I think we have at least the following options:

1. Make the existing annotations @Inherited
2. Add a variant for each annotation that is @Inherited
3. Change the annotation search algorithm for test classes to check parent classes as well

Both (1) and (3) would be breaking changes. For (2), I think we could generate the @Inherited variant, and add some glue code that uses a java.lang.reflect.Proxy so the majority of the code wouldn't have to change.

Any other ideas?

[scordio]

Change the annotation search algorithm for test classes to check parent classes as well

To avoid the breaking change, can this be combined with a new opt-in flag for the current annotations, false by default? Something like in inherited=true or similar.

[marcphilipp]

Team decision: Wait for additional interest from the community.

[vdmitrienko]

+1 upvote

I recently faced a similar use case with the @DisabledOnOs annotation. I was 99% sure I could simply place it on a base class, but discovered that the annotation is not inherited.

The approach suggested by @scordio looks great to me.

@marcphilipp @scordio, if you don't mind, I’d be happy to submit a PR if you decide to implement this.

[sbrannen]

To avoid the breaking change, can this be combined with a new opt-in flag for the current annotations, false by default? Something like in inherited=true or similar.

That would certainly be an option, and it's similar to what I did for several test-related annotations in the Spring Framework (for example, @TestExecutionListeners(inheritListeners = false), @ActiveProfiles(inheritProfiles = false), @ContextConfiguration(inheritLocations = false), @ContextCustomizerFactories(inheritFactories = false) etc.).

However, there's a difference: all of those inherit* flags in Spring default to true, and that changes the programming model slightly. But you'd end up with effectively the same level of opt-in support.

If we introduce inherit* flags in JUnit Jupiter, they would have to be set manually for all conditional annotations. So, another option would be to introduce something like @InheritConditions (with a boolean value() default true attribute) that applies to all conditional annotations on the current type and its subtypes. @InheritConditions could of course be declared on a subtype to override the inherited @InheritConditions configuration.

I'm simply "playing devil's advocate" here to ensure we think about multiple use cases.

[sbrannen]

+1 upvote

@vdmitrienko, in case you meant to up-vote this feature request, we use the 👍 emoji "reaction" on the issue description to track that.

[vdmitrienko]

@vdmitrienko, in case you meant to up-vote this feature request, we use the 👍 emoji "reaction" on the issue description to track that.

Thanks @sbrannen !

[vdmitrienko]

That would certainly be an option, and it's similar to what I did for several test-related annotations in the Spring Framework (for example, @TestExecutionListeners(inheritListeners = false), @ActiveProfiles(inheritProfiles = false), @ContextConfiguration(inheritLocations = false), @ContextCustomizerFactories(inheritFactories = false) etc.).

@sbrannen, this brings me to the following thoughts:

Perhaps, from a maintenance standpoint, using @InheritConditions could be a slightly better option. If the "conditional" annotations default to inherited=false, then, for consistency, any future annotations with such a flag should also default to false. However, with @InheritConditions, there is no obligation to maintain that consistency, providing the flexibility to use inherited=true as the default in the future.