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

Application Context initialized twice during test when exception thrown during initialization #24888

Closed
QwertGold opened this issue Jan 19, 2021 · 12 comments
Labels
type: bug A general bug
Milestone

Comments

@QwertGold
Copy link

I noticed that when I have a Spring Boot test, and the context fails to initialize, the Banner is printed twice, and I decided to look into why.

// spring-boot-test-autoconfigure 2.3.7.RELEASE
public class SpringBootDependencyInjectionTestExecutionListener extends DependencyInjectionTestExecutionListener {

	@Override
	public void prepareTestInstance(TestContext testContext) throws Exception {
		try {
			super.prepareTestInstance(testContext);
		}
		catch (Exception ex) {
			outputConditionEvaluationReport(testContext);
			throw ex;
		}
	}
.....
}

When an exception is thrown during initialization, outputConditionEvaluationReport(testContext) is called, which eventually leads to the context being initialized a second time inside DefaultCacheAwareContextLoaderDelegate::loadContext.

This is more a nuisance than a bug, as it only happens in test code; the cost is 'just' developer time and build resources.

I this particular case I want to fail fast, so I throw an exception because I know bad things will happen later.
I'm trying to safeguard against less experienced developers, using resources outside the intended lifecycle. During tests we automatically create external resources in an early lifecycle phase, these resources may only be accessed during later phases, and after the application is started. If a developer violate this rule, and tries to access a resource during bean construction or in an earlier lifecycle, I can detect it, and throw an IlligealStateException. If I don't do this I will get an exception later from the resource Api, and it can be hard for junior developers to identify that they have violated the resource lifecycle, if they get a 404/500 error from a HttpClient.

One possible solution, is to have an AbortTestContextInitializationException, that you could throw, if you decide that the TestContext is in a state where it does not make sense to continue executing, and there is no point in trying to generate the ConditionEvaluationReport. Ideally the failure of the initialization would be cached, so other tests using the same context would fail immediately instead of using build resources , trying to create the context twice for every test.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 19, 2021
@philwebb
Copy link
Member

@QwertGold I think I understand the description, but it would be useful if you could provide a sample application that shows the problem.

@philwebb philwebb added the status: waiting-for-feedback We need additional information before we can continue label Jan 19, 2021
@QwertGold
Copy link
Author

Cool, let me build a small project to illustrate

@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 Jan 19, 2021
@QwertGold
Copy link
Author

I'm unable to reproduce this in a simple stand alone project.
When I debug it I can see some differences in the call stack depth at the point where the ApplicationContext is created, so I will need some time to figure out where why my larger project behaves differently, maybe I'll learn something along the way - I usually do ;)

@mbhave mbhave added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jan 19, 2021
@QwertGold
Copy link
Author

I figure out how to reproduce this, it seems to happen when the test has a real Web server environment @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)

I started with JUnit 4 and boot 2.3.7 as this is what we use, but the behavior is the same for Junit5 and boot 2.4.2, the repo has commits for both cases, https://github.com/QwertGold/spring-24888

@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 Jan 20, 2021
@rokii
Copy link

rokii commented Aug 20, 2021

hit same issue , any update ?

I suggest adding below in the catch block{}

catch (Exception ex) {
     if(!(ex.getCause() instanceof UnsatisfiedDependencyException)){
       outputConditionEvaluationReport(testContext);
     }
     throw ex;
}

Or

catch (Exception ex) {
     if(testContext.hasApplicationContext()){
       outputConditionEvaluationReport(testContext);
     }
     throw ex;
}

@wilkinsona wilkinsona changed the title Application Context initialized twice, during test when exception thrown during initialization Application Context initialized twice during test when exception thrown during initialization Oct 22, 2021
@wilkinsona
Copy link
Member

Thanks for the sample and for your patience while we found time to start looking at this, @QwertGold.

The problem doesn't occur with a mock web environment (plain @SpringBootTest) because the context refresh is triggered earlier (by ServletTestExecutionListener) which we don't replace so no attempt is made to output the condition evaluation report.

When there's a full-blown web environment, ServletTestExecutionListener does not run because the org.springframework.test.context.web.ServletTestExecutionListener.activateListener attribute in the test context has been set to false. This allows preparation of the test instance to proceed and to reach SpringBootDependencyInjectionTestExecutionListener which triggers the double initialization when trying to output the condition evaluation report after the refresh failure.

@wilkinsona
Copy link
Member

The behaviour's the same back in 1.4.0-M2 when the printing of the condition evaluation report was first introduced (see #4901 and e5f2241) and in 1.4.1 when it was refined (see #6874 and 7134586).

When refresh fails the context won't be available so I can't see how the current approach will be able to access the context to generate the report. We could just stop using SpringBootDependencyInjectionTestExecutionListener as it doesn't appear to work as hoped or I think we need to rework the approach in a way that means we don't need to get the application context from the test context to trigger the generation of the report.

Flagging for an upcoming team meeting so that we can discuss what to do.

@wilkinsona wilkinsona added for: team-meeting An issue we'd like to discuss as a team to make progress type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Oct 22, 2021
@wilkinsona wilkinsona added this to the 2.4.x milestone Oct 22, 2021
@philwebb philwebb added status: pending-design-work Needs design work before any code can be developed and removed for: team-meeting An issue we'd like to discuss as a team to make progress labels Nov 10, 2021
@wilkinsona wilkinsona modified the milestones: 2.4.x, 2.5.x Nov 17, 2021
@mirceade

This comment has been minimized.

@wilkinsona wilkinsona modified the milestones: 2.5.x, 2.6.x May 19, 2022
@mikelhamer
Copy link

mikelhamer commented Jul 14, 2022

Just pulled my hair out trying to figure out why my context was loading twice as I was trying to figure out what was causing it to have an error during initialization in the first place.....not fun!

@sbrannen
Copy link
Member

We could just stop using SpringBootDependencyInjectionTestExecutionListener as it doesn't appear to work as hoped or I think we need to rework the approach in a way that means we don't need to get the application context from the test context to trigger the generation of the report.

I'm favor of getting rid of SpringBootDependencyInjectionTestExecutionListener and replacing it with a dedicated mechanism.

What do you think about introducing an SPI (in Spring Framework 6.0) in the TestContext framework for "processing" ApplicationContext load failures -- basically a new interface that Boot could implement and register (potentially via the spring.factories mechanism)?

Related Issues:

@wilkinsona
Copy link
Member

That sounds great, Sam. Thanks. We'll be left with the problem described in this issue in 2.x, but with a much better path forward in 3.0.

@sbrannen
Copy link
Member

That sounds great, Sam. Thanks. We'll be left with the problem described in this issue in 2.x, but with a much better path forward in 3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

9 participants