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

@RecordApplicationEvents doesn't work on threads spawned before the test execution #31079

Closed
romainmoreau opened this issue Aug 20, 2023 · 4 comments
Assignees
Labels
in: test Issues in the test module status: declined A suggestion or change that we don't feel we should currently apply

Comments

@romainmoreau
Copy link

romainmoreau commented Aug 20, 2023

The issue with ApplicationEventsHolder and its InheritableThreadLocal solution implemented in #30020 is that it only works if the thread is spawned in the test method like in the unit test JUnit4ApplicationEventsAsyncIntegrationTests but if the thread is spawned before registerApplicationEvents is called on the main/parent thread (for instance during application context creation), then every application event emitted during the test on this spawned thread will be ignored.

Here's a slightly modified version of JUnit4ApplicationEventsAsyncIntegrationTests that reproduce the issue:

@SpringBootTest
@RecordApplicationEvents
public class JUnit4ApplicationEventsAsyncIntegrationTests {
	@Autowired
	ApplicationEvents applicationEvents;

	@Autowired
	Thread thread;

	@Test
	public void asyncPublication() throws InterruptedException {
		thread.start();
		thread.join();

		assertThat(this.applicationEvents.stream(CustomEvent.class)).singleElement()
				.extracting(CustomEvent::getMessage, InstanceOfAssertFactories.STRING).isEqualTo("async");
	}

	@Configuration
	static class Config {
		@Autowired
		ApplicationContext context;

		@Bean
		Thread thread() {
			return new Thread(() -> context.publishEvent(new CustomEvent("async")));
		}
	}

	static class CustomEvent extends ApplicationEvent {
		private static final long serialVersionUID = 1L;

		private final String message;

		CustomEvent(String message) {
			super(message);
			this.message = message;
		}

		String getMessage() {
			return message;
		}
	}
}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 20, 2023
@simonbasle simonbasle self-assigned this Nov 7, 2023
@simonbasle
Copy link
Contributor

I don't think there is a perfect solution for your use case beyond what was introduced in #30020.

Perhaps the scope of @RecordApplicationEvents should be clarified to state it only includes events published from the unit test's thread or any thread that is a child of the unit test thread?

As a possible workaround however, maybe you could have your @Configuration class instantiate a Runnable instead of a Thread? That way, your test method can spawn the Thread itself, approximating the production behavior with a working recording during the test.

@simonbasle simonbasle added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 7, 2023
@simonbasle simonbasle added this to the 6.1.x milestone Nov 7, 2023
@simonbasle simonbasle added the in: test Issues in the test module label Nov 7, 2023
@romainmoreau
Copy link
Author

Unfortunately my real use case was a @Bean that creates threads during its instanciation and that is outside of my scope (provided by a library).

I finally moved on from @RecordApplicationEvents and used instead a simple @Component with an @EventListener method that records events.

@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 Nov 7, 2023
simonbasle added a commit that referenced this issue Nov 9, 2023
This commit clarifies that the annotation is used to capture events that
are fired from the test `Thread` or descendant threads, since the test
framework uses `InheritableThreadLocal` to store captured events now.

See gh-31079
See gh-30020
@simonbasle
Copy link
Contributor

yeah that makes sense that the real setup is a little bit more complex.

the test recording of events is aiming at capturing events specific to a given test, first and foremost, in order to let users assert a shorter scope of test-fired events exhaustively without being overwhelmed by unrelated events. glad you found a creative workaround solution for your test.

I've added a short note in the javadoc of @RecordApplicationEvents to clarify a bit however, see 4287417.

@simonbasle simonbasle closed this as not planned Won't fix, can't repro, duplicate, stale Nov 9, 2023
@simonbasle simonbasle removed this from the 6.1.x milestone Nov 9, 2023
@simonbasle simonbasle added status: declined A suggestion or change that we don't feel we should currently apply and removed status: feedback-provided Feedback has been provided labels Nov 9, 2023
@joshiste
Copy link
Contributor

joshiste commented Jun 5, 2024

I just ran into the same issue. Now rolling my own version of this, which I think is a pity.

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
Projects
None yet
Development

No branches or pull requests

4 participants