Skip to content

Find all local @TestExecutionListeners annotations on a test class #26141

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
stolsvik opened this issue Nov 23, 2020 · 7 comments
Closed

Find all local @TestExecutionListeners annotations on a test class #26141

stolsvik opened this issue Nov 23, 2020 · 7 comments
Labels
in: test Issues in the test module status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@stolsvik
Copy link

stolsvik commented Nov 23, 2020

When using TestExecutionListeners, I have multiple times ended up getting severely hampered by the fact that there can (evidently) only be one such annotation per test run - any more, and the latest supersedes the previous.

This is unfortunate as the TestExecutionListener can be used as a meta-annotation. I've tried to make some nice bespoke annotations that can be used in our shop that can be slapped on the test case - but that fails if the test also includes a @TestExecutionListeners annotation - or a second bespoke annotation that also needs to add itself as a TestExecutionListener.

I don't quite see how this would fit into the existing annotation (as the two modes there - REPLACE_DEFAULTS and MERGE_WITH_DEFAULTS - might not be open for further extension?). Maybe a new AppendTestExecutionListeners?

See also #26142

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 23, 2020
@sbrannen sbrannen added in: test Issues in the test module type: enhancement A general enhancement labels Nov 24, 2020
@sbrannen sbrannen changed the title Test framework: TestExecutionListener "append", not replace or merge. Find all local @TestExecutionListeners annotations on a test class Nov 24, 2020
@sbrannen
Copy link
Member

Thanks for raising the issue.

I have created #26145 which is a similar request for @ActiveProfiles, and we will consider this in conjunction with that issue.

@stolsvik
Copy link
Author

stolsvik commented Nov 24, 2020

@sbrannen I am not sure I agree to the title change? Or, I might not understand what you mean by "local".

For my part, I wish the test-framework would "collate" all @TestExecutionListeners in the scope of the test.

Specifically, I would like to use the @TestExecutionListeners as a meta-annotation on an annotation @SpringInjectRules which fixes injection of JUnit Rules, in my "Mats" library. The problem is if the test-class also specifies TestExecutionListeners, then my annotation's wishes simply vanishes.

I find such an annotation much nicer and succinct than explaining that you must write this on your test class:
@TestExecutionListeners(listeners = SpringInjectRulesTestExecutionListener.class, mergeMode = MergeMode.MERGE_WITH_DEFAULTS)

The reason for mentioning a new annotation, is that I do not see how you would get this to play nice with the existing options REPLACE_DEFAULTS and MERGE_WITH_DEFAULTS. This new @AppendTestExecutionListeners should append no matter what - the reason the user employed this annotation directly, OR used an annotation that was meta-annotated, was that he actually wanted this TestExecutionListener to run, not that it might "randomly" be replaced with another list! (I bet this REPLACE_DEFAULTS default has tricked pretty many users, a Google search at least reveals as much).

@sbrannen
Copy link
Member

sbrannen commented Nov 24, 2020

When I say "local", I mean all such annotations that are directly present or meta-present on the test class (i.e., excluding any such annotations present on an implemented interface or present on a superclass or enclosing class).

In Spring Framework 5.2, we added similar support for @TestPropertySource by making it a @Repeatable annotation. Thus what I have in mind here (if we decide to implement such a feature) is to provide similar support for the @TestExecutionListeners annotation. Please see the issue description and example in #26145 for further information.

One thing to point out is that all "local" declarations of @TestExecutionListeners would effectively be merged together as if there were only one @TestExecutionListeners declaration containing all of the configured listeners for the current test class. That's precisely what we do for @TestPropertySource.

@cdalexndr
Copy link

My use case is bundling a set of test annotations inside my custom annotation:


@TestExecutionListeners({MyFeatureTestExecutionListener.class})
... <other annotation>
public @interface EnableSomeTestFeature {

but using it on a test class that also has @TestExecutionListeners fails:

@SpringBootTest
@EnableSomeTestFeature 
@TestExecutionListeners({MockitoTestExecutionListener.class,ResetMocksTestExecutionListener.class})
pubclic class MyTestClass{

MyFeatureTestExecutionListener is not registered.

Spring already has support for aggregating multiple @TestExecutionListeners but only when using super classes:

// Traverse the class hierarchy...
while (descriptor != null) {
Class<?> declaringClass = descriptor.getDeclaringClass();
TestExecutionListeners testExecutionListeners = descriptor.getAnnotation();
if (logger.isTraceEnabled()) {
logger.trace(String.format("Retrieved @TestExecutionListeners [%s] for declaring class [%s].",
testExecutionListeners, declaringClass.getName()));
}
boolean inheritListeners = testExecutionListeners.inheritListeners();
AnnotationDescriptor<TestExecutionListeners> parentDescriptor = descriptor.next();
// If there are no listeners to inherit, we might need to merge the
// locally declared listeners with the defaults.
if ((!inheritListeners || parentDescriptor == null) &&
testExecutionListeners.mergeMode() == MergeMode.MERGE_WITH_DEFAULTS) {
if (logger.isDebugEnabled()) {
logger.debug(String.format("Merging default listeners with listeners configured via " +
"@TestExecutionListeners for class [%s].", descriptor.getRootDeclaringClass().getName()));
}
usingDefaults = true;
classesList.addAll(getDefaultTestExecutionListenerClasses());
}
classesList.addAll(0, Arrays.asList(testExecutionListeners.listeners()));
descriptor = (inheritListeners ? parentDescriptor : null);
}

Making @TestExecutionListeners annotation repeatable will fix this issue, and it should be straight forward reusing the same logic above.

@sbrannen
Copy link
Member

sbrannen commented Jun 7, 2022

Since the community has not displayed significant interest in this feature during the last 1.5 years, the team has decided to close this issue.

However, if the community displays increased interest in this feature we would be willing to reconsider.

@sbrannen sbrannen closed this as not planned Won't fix, can't repro, duplicate, stale Jun 7, 2022
@sbrannen sbrannen added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 7, 2022
@stolsvik
Copy link
Author

stolsvik commented Jun 7, 2022

I find this sad - I believe this is one of those issues where devs end up with "oh, that doesn't work - we'll have to hack our way around it then", typically after hours of head-scratching.

It seems like the feature would be pretty straight-forward to implement, as similar features evidently exists which do handle the @TestPropertySource, ref. #23299. It would close a very annoying, and obvious when you hit it, limitation with the framework.

@stolsvik
Copy link
Author

stolsvik commented Jun 7, 2022

It also seems like #26141, #26142, and #26145 would be fixed in an identical manner - I am certain that the combined annoyance with these deficiencies are pretty substantial if you were able to sum it up.

@sbrannen sbrannen changed the title Find all local @TestExecutionListeners annotations on a test class Find all local @TestExecutionListeners annotations on a test class Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants