Skip to content

Commit 9870ce1

Browse files
sbrannensbrannen
committed
Only update ObservationThreadLocalAccessor when test has an active ApplicationContext
Prior to this commit, MicrometerObservationRegistryTestExecutionListener always attempted to load the test's ApplicationContext in order to update the ObservationThreadLocalAccessor in its beforeTestMethod() callback, even if there was no active ApplicationContext. To avoid unnecessarily loading an ApplicationContext or attempting to load an ApplicationContext that cannot be loaded (for example, due to a context-load failure), this commit applies a hasApplicationContext() check in beforeTestMethod(). Since the MicrometerObservationRegistryTestExecutionListener is registered after the DependencyInjectionTestExecutionListener (at least by default), an active ApplicationContext should be present unless dependency injection from the context failed or the context failed to load. Closes gh-36817
1 parent d87c03a commit 9870ce1

4 files changed

Lines changed: 240 additions & 4 deletions

File tree

‎spring-test/src/main/java/org/springframework/test/context/observation/MicrometerObservationRegistryTestExecutionListener.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@
3636
*
3737
* <p>This listener updates the {@link ObservationThreadLocalAccessor} with the
3838
* {@code ObservationRegistry} obtained from the test's {@link ApplicationContext},
39-
* if present.
39+
* if the {@code ApplicationContext} is available and contains an
40+
* {@code ObservationRegistry} bean.
4041
*
4142
* @author Marcin Grzejszczak
4243
* @author Sam Brannen
@@ -125,9 +126,10 @@ public final int getOrder() {
125126
}
126127

127128
/**
128-
* If the test's {@link ApplicationContext} contains an {@link ObservationRegistry}
129-
* bean, this method retrieves the {@code ObservationRegistry} currently stored
130-
* in {@link ObservationThreadLocalAccessor}, saves a reference to the original
129+
* If the test's {@link ApplicationContext} is available and contains an
130+
* {@link ObservationRegistry} bean, this method retrieves the
131+
* {@code ObservationRegistry} currently stored in
132+
* {@link ObservationThreadLocalAccessor}, saves a reference to the original
131133
* registry as a {@link TestContext} attribute (to be restored in
132134
* {@link #afterTestMethod(TestContext)}), and sets the registry from the test's
133135
* {@code ApplicationContext} in {@link ObservationThreadLocalAccessor}.
@@ -136,6 +138,9 @@ public final int getOrder() {
136138
*/
137139
@Override
138140
public void beforeTestMethod(TestContext testContext) {
141+
if (!testContext.hasApplicationContext()) {
142+
return;
143+
}
139144
testContext.getApplicationContext().getBeanProvider(ObservationRegistry.class)
140145
.ifAvailable(registry -> {
141146
if (logger.isDebugEnabled()) {

‎spring-test/src/test/java/org/springframework/test/context/observation/MicrometerObservationRegistryTestExecutionListenerTests.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,17 @@
3434
import static org.mockito.BDDMockito.given;
3535
import static org.mockito.BDDMockito.willAnswer;
3636
import static org.mockito.Mockito.mock;
37+
import static org.mockito.Mockito.never;
38+
import static org.mockito.Mockito.verify;
3739

3840
/**
3941
* Tests for {@link MicrometerObservationRegistryTestExecutionListener}.
4042
*
4143
* @author Marcin Grzejszczak
4244
* @author Sam Brannen
4345
* @since 6.0.10
46+
* @see MicrometerObservationRegistryTestExecutionListenerWithContextLoadFailureTests
47+
* @see MicrometerObservationRegistryTestExecutionListenerWithContextLoadFailureTestNGTests
4448
*/
4549
class MicrometerObservationRegistryTestExecutionListenerTests {
4650

@@ -62,11 +66,31 @@ void configureTestContextMock() {
6266
.given(testContext).setAttribute(anyString(), any());
6367
given(testContext.removeAttribute(anyString()))
6468
.willAnswer(invocation -> attributes.get(invocation.getArgument(0, String.class)));
69+
given(testContext.hasApplicationContext()).willReturn(true);
6570
given(testContext.getApplicationContext()).willReturn(applicationContext);
6671
Class testClass = getClass();
6772
given(testContext.getTestClass()).willReturn(testClass);
6873
}
6974

75+
@Test // gh-36817
76+
void beforeTestMethodIsNoOpWhenContextIsNotAvailable() throws Exception {
77+
given(testContext.hasApplicationContext()).willReturn(false);
78+
79+
listener.beforeTestMethod(testContext);
80+
81+
verify(testContext).hasApplicationContext();
82+
verify(testContext, never()).getApplicationContext();
83+
}
84+
85+
@Test // gh-36817
86+
void afterTestMethodIsNoOpWhenContextIsNotAvailable() throws Exception {
87+
given(testContext.hasApplicationContext()).willReturn(false);
88+
89+
listener.afterTestMethod(testContext);
90+
91+
verify(testContext, never()).getApplicationContext();
92+
}
93+
7094
@Test
7195
void observationRegistryIsNotOverridden() throws Exception {
7296
assertGlobalObservationRegistryIsSameAsOriginal();

‎spring-test/src/test/java/org/springframework/test/context/observation/MicrometerObservationRegistryTestExecutionListenerWithContextLoadFailureTestNGTests.java‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/*
2+
* Copyright 2002-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.test.context.observation;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.testng.TestNG;
21+
22+
import org.springframework.context.annotation.Bean;
23+
import org.springframework.context.annotation.Configuration;
24+
import org.springframework.test.context.ContextConfiguration;
25+
import org.springframework.test.context.TestExecutionListeners;
26+
import org.springframework.test.context.support.DependencyInjectionTestExecutionListener;
27+
import org.springframework.test.context.testng.AbstractTestNGSpringContextTests;
28+
import org.springframework.test.context.testng.TrackingTestNGTestListener;
29+
30+
import static org.assertj.core.api.Assertions.assertThat;
31+
32+
/**
33+
* JUnit based integration test which verifies that
34+
* {@link MicrometerObservationRegistryTestExecutionListener} &mdash; when used in
35+
* conjunction with Spring's TestNG support &mdash; does not attempt to load an
36+
* application context to update the
37+
* {@link io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor}
38+
* if the application context is not currently loaded or previously failed to load.
39+
*
40+
* @author Sam Brannen
41+
* @since 7.1
42+
* @see <a href="https://github.com/spring-projects/spring-framework/issues/36817">gh-36817</a>
43+
* @see MicrometerObservationRegistryTestExecutionListenerWithContextLoadFailureTests
44+
*/
45+
class MicrometerObservationRegistryTestExecutionListenerWithContextLoadFailureTestNGTests {
46+
47+
/**
48+
* <p>NOTE: The {@code @BeforeMethod(alwaysRun = true)} and {@code @AfterMethod(alwaysRun = true)}
49+
* lifecycle methods in {@link AbstractTestNGSpringContextTests} are always invoked, even if a
50+
* previous lifecycle configuration method failed (for example, due to a context-load failure).
51+
*/
52+
@Test
53+
void contextLoadFailureCausesExpectedTestFailures() {
54+
TrackingTestNGTestListener listener = new TrackingTestNGTestListener();
55+
TestNG testNG = new TestNG();
56+
testNG.addListener(listener);
57+
testNG.setTestClasses(new Class<?>[] { ContextLoadFailureTestCase.class });
58+
testNG.setVerbose(0);
59+
testNG.run();
60+
61+
assertThat(listener.testStartCount).as("tests started").hasValue(2);
62+
assertThat(listener.testSuccessCount).as("tests succeeded").hasValue(0);
63+
assertThat(listener.testFailureCount).as("tests failed").hasValue(0);
64+
// Before the introduction of the hasApplicationContext() check in
65+
// MicrometerObservationRegistryTestExecutionListener, the @BeforeMethod lifecycle
66+
// method in AbstractTestNGSpringContextTests also attempted to load the faulty
67+
// ApplicationContext, resulting in 3 configuration failures:
68+
// 1 * @BeforeClass + 2 * @BeforeMethod = 3.
69+
// With the fix, only the @BeforeClass context-load failure is recorded.
70+
assertThat(listener.failedConfigurationsCount).as("failed configurations").hasValue(1);
71+
}
72+
73+
74+
/**
75+
* <p>The {@code @TestExecutionListeners} declaration replaces the default listeners with
76+
* only those needed to exercise {@link MicrometerObservationRegistryTestExecutionListener},
77+
* ensuring that no additional listeners call {@code testContext.getApplicationContext()}
78+
* without a conditional {@code hasApplicationContext()} check.
79+
*/
80+
@ContextConfiguration
81+
@TestExecutionListeners({
82+
DependencyInjectionTestExecutionListener.class,
83+
MicrometerObservationRegistryTestExecutionListener.class
84+
})
85+
static class ContextLoadFailureTestCase extends AbstractTestNGSpringContextTests {
86+
87+
@org.testng.annotations.Test
88+
void test1() {
89+
}
90+
91+
@org.testng.annotations.Test
92+
void test2() {
93+
}
94+
95+
@Configuration(proxyBeanMethods = false)
96+
static class Config {
97+
98+
@Bean
99+
String alwaysFails() {
100+
throw new RuntimeException("Simulated context load failure");
101+
}
102+
}
103+
}
104+
105+
}

‎spring-test/src/test/java/org/springframework/test/context/observation/MicrometerObservationRegistryTestExecutionListenerWithContextLoadFailureTests.java‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/*
2+
* Copyright 2002-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.test.context.observation;
18+
19+
import org.junit.jupiter.api.MethodOrderer.MethodName;
20+
import org.junit.jupiter.api.Test;
21+
import org.junit.jupiter.api.TestMethodOrder;
22+
import org.junit.platform.testkit.engine.EngineTestKit;
23+
24+
import org.springframework.context.annotation.Bean;
25+
import org.springframework.context.annotation.Configuration;
26+
import org.springframework.test.context.TestExecutionListeners;
27+
import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
28+
import org.springframework.test.context.support.DependencyInjectionTestExecutionListener;
29+
30+
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
31+
import static org.junit.platform.testkit.engine.EventConditions.event;
32+
import static org.junit.platform.testkit.engine.EventConditions.finishedWithFailure;
33+
import static org.junit.platform.testkit.engine.EventConditions.test;
34+
import static org.junit.platform.testkit.engine.TestExecutionResultConditions.instanceOf;
35+
import static org.junit.platform.testkit.engine.TestExecutionResultConditions.message;
36+
37+
/**
38+
* Tests which indirectly verify that {@link MicrometerObservationRegistryTestExecutionListener}
39+
* does not attempt to load an application context to update the
40+
* {@link io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor} if
41+
* the application context is not currently loaded or previously failed to load.
42+
*
43+
* @author Sam Brannen
44+
* @since 7.1
45+
* @see <a href="https://github.com/spring-projects/spring-framework/issues/36817">gh-36817</a>
46+
* @see MicrometerObservationRegistryTestExecutionListenerTests
47+
* @see MicrometerObservationRegistryTestExecutionListenerWithContextLoadFailureTestNGTests
48+
*/
49+
class MicrometerObservationRegistryTestExecutionListenerWithContextLoadFailureTests {
50+
51+
@Test
52+
void contextLoadFailureCausesExpectedTestFailures() {
53+
EngineTestKit.engine("junit-jupiter")
54+
.selectors(selectClass(ContextLoadFailureTestCase.class))
55+
.execute()
56+
.testEvents()
57+
.assertStatistics(stats -> stats.started(2).succeeded(0).failed(2))
58+
.assertThatEvents()
59+
.haveExactly(1, event(test("test1"),
60+
finishedWithFailure(
61+
instanceOf(IllegalStateException.class),
62+
message(msg -> msg.startsWith("Failed to load ApplicationContext")))))
63+
.haveExactly(1, event(test("test2"),
64+
finishedWithFailure(
65+
instanceOf(IllegalStateException.class),
66+
message(msg -> msg.contains("failure threshold")))));
67+
}
68+
69+
70+
/**
71+
* <p>The {@code @TestExecutionListeners} declaration replaces the default listeners with
72+
* only those needed to exercise {@link MicrometerObservationRegistryTestExecutionListener},
73+
* ensuring that no additional listeners call {@code testContext.getApplicationContext()}
74+
* without a conditional {@code hasApplicationContext()} check.
75+
*/
76+
@SpringJUnitConfig
77+
@TestExecutionListeners({
78+
DependencyInjectionTestExecutionListener.class,
79+
MicrometerObservationRegistryTestExecutionListener.class
80+
})
81+
@TestMethodOrder(MethodName.class)
82+
static class ContextLoadFailureTestCase {
83+
84+
@Test
85+
void test1() {
86+
}
87+
88+
@Test
89+
void test2() {
90+
}
91+
92+
@Configuration(proxyBeanMethods = false)
93+
static class Config {
94+
95+
@Bean
96+
String alwaysFails() {
97+
throw new RuntimeException("Simulated context load failure");
98+
}
99+
}
100+
}
101+
102+
}

0 commit comments

Comments
 (0)