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

Enable default TestExecutionListeners in JUnit 4 and TestNG base test classes #29149

Closed
oopschen opened this issue Sep 14, 2022 · 3 comments
Closed
Assignees
Labels
in: test Issues in the test module type: enhancement A general enhancement
Milestone

Comments

@oopschen
Copy link

As spring test framework support testExecutionListener automatic discovery, when i build project using spring security with testng, it can not load META-INF/spring.factories file in org.springframework.security:spring-security-test library.(Same as Junit4, but works for junit5).

May affected spring version

since 4.1

After searching the source code of spring test framework, below is how it happened:

spring test framework testExecutionListener automatic discovery

Please check file org.springframework.test.context.support.AbstractTestContextBootstrapper at method getTestExecutionListeners.
The method prefer to load @TestExecutionListeners over META-INF/spring.factories , only if no @TestExecutionListeners annotation found, it loads default execution listeners(META-INF/spring.factories ).

IMPORTANT CODE FROM SPRING FRAMEWORK

                                // 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());
				}

As the code show, it must have testExecutionListeners.mergeMode() set to MergeMode.MERGE_WITH_DEFAULTS and either it has no super class or set testExecutionListeners.inheritListeners to false.

TestNG support

TestNG support spring test framework througth AbstractTestNGSpringContextTests and AbstractTransactionalTestNGSpringContextTests classes, both class annotated by @TestExecutionListeners annotation.
With the default value: inheritListeners set to true, mergeMode set to MergeMode.REPLACE_DEFAULTS

The Cause

Any class inherit AbstractTestNGSpringContextTests and AbstractTransactionalTestNGSpringContextTests will not load the default execution listeners(META-INF/spring.factories ). It will fail to take advantage of testExecutionListener automatic discovery mechanism, the same to Junit4.

Temporary solution

Annotate class inheriting AbstractTestNGSpringContextTests :

@TestExecutionListeners(value = {ServletTestExecutionListener.class,
    DirtiesContextBeforeModesTestExecutionListener.class,
    ApplicationEventsTestExecutionListener.class, DependencyInjectionTestExecutionListener.class,
    DirtiesContextTestExecutionListener.class, EventPublishingTestExecutionListener.class
}, mergeMode = MergeMode.MERGE_WITH_DEFAULTS, inheritListeners = false)

Solution Advice

  1. Better to remove @TestExecutionListeners annotation in AbstractTestNGSpringContextTests , AbstractTransactionalTestNGSpringContextTests, AbstractTransactionalJUnit4SpringContextTests and AbstractTransactionalJUnit4SpringContextTests, let the project choose to use TestExecutionListeners through testExecutionListener automatic discovery mechanism.
  2. Add new Composed Annotation to let the project use in any test class.
  3. change TestExecutionListeners default values in AbstractXXXXSpringContextTests classes: inheritListeners set to false, mergeMode set to MergeMode.MERGE_WITH_DEFAULTS
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 14, 2022
@sbrannen sbrannen added the in: test Issues in the test module label Sep 14, 2022
@sbrannen
Copy link
Member

Temporary solution

Annotate class inheriting AbstractTestNGSpringContextTests :

@TestExecutionListeners(value = {ServletTestExecutionListener.class,
    DirtiesContextBeforeModesTestExecutionListener.class,
    ApplicationEventsTestExecutionListener.class, DependencyInjectionTestExecutionListener.class,
    DirtiesContextTestExecutionListener.class, EventPublishingTestExecutionListener.class
}, mergeMode = MergeMode.MERGE_WITH_DEFAULTS, inheritListeners = false)

What happens if you use the following instead?

@TestExecutionListeners(listeners = {}, inheritListeners = false, mergeMode = MergeMode.MERGE_WITH_DEFAULTS)

That should allow you to switch to using all default listeners.

@sbrannen sbrannen added the status: waiting-for-feedback We need additional information before we can continue label Sep 14, 2022
@sbrannen sbrannen changed the title Improve testExecutionListener automatic discovery support for Junit4 & Testng Improve TestExecutionListener automatic discovery support TestNG Sep 14, 2022
@sbrannen sbrannen changed the title Improve TestExecutionListener automatic discovery support TestNG Improve TestExecutionListener automatic discovery support for TestNG Sep 14, 2022
@oopschen
Copy link
Author

Temporary solution

Annotate class inheriting AbstractTestNGSpringContextTests :

@TestExecutionListeners(value = {ServletTestExecutionListener.class,
    DirtiesContextBeforeModesTestExecutionListener.class,
    ApplicationEventsTestExecutionListener.class, DependencyInjectionTestExecutionListener.class,
    DirtiesContextTestExecutionListener.class, EventPublishingTestExecutionListener.class
}, mergeMode = MergeMode.MERGE_WITH_DEFAULTS, inheritListeners = false)

What happens if you use the following instead?

@TestExecutionListeners(listeners = {}, inheritListeners = false, mergeMode = MergeMode.MERGE_WITH_DEFAULTS)

That should allow you to switch to using all default listeners.

Thanks for the reply.
It works, better to make the AbstractTestNGSpringContextTests utilize the testExecutionListener automatic discovery mechanism(official document mention). It requires some work to find the truth while the official document haven't mention it.
Beginners fails to start a project by integrating 3rd party library using testExecutionListener automatic discovery mechanism with TestNG or Junit4(That's how i find it).

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 15, 2022
@sbrannen
Copy link
Member

sbrannen commented Oct 7, 2022

Hi @oopschen,

Thanks for the feedback.

For 6.0 we will remove the @TestExecutionListeners on those 4 base classes to allow users to benefit from default TestExecutionListener registration.

Since that may potentially be a breaking change for some users, we will only update the documentation for 5.3.x. I've created #29281 to address that.

@sbrannen sbrannen changed the title Improve TestExecutionListener automatic discovery support for TestNG Enable default TestExecutionListeners in JUnit 4 and TestNG base test classes Oct 7, 2022
@sbrannen sbrannen added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Oct 7, 2022
@sbrannen sbrannen self-assigned this Oct 7, 2022
@sbrannen sbrannen added this to the 6.0.0-RC1 milestone Oct 7, 2022
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 type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants