Skip to content

Commit 0ee15f0

Browse files
mpkorstanjempkorstanje
authored
[Spring] Cleanly stop after failure to start application context (#2570)
Fixes: #2569
1 parent 2442361 commit 0ee15f0

File tree

4 files changed

+123
-64
lines changed

4 files changed

+123
-64
lines changed

‎CHANGELOG.md

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
88
## [Unreleased] (In Git)
99

1010
### Added
11+
* [Core] Warn when glue path is passed as file scheme instead of classpath ([#2547](https://github.com/cucumber/cucumber-jvm/pull/2547) M.P. Korstanje)
1112

1213
### Changed
1314

@@ -16,12 +17,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
1617
### Removed
1718

1819
### Fixed
20+
* [Spring] Cleanly stop after failure to start application context
1921

20-
## [7.3.5] (2022-05-10)
21-
22-
### Added
23-
* [Core] Warn when glue path is passed as file scheme instead of classpath ([#2547](https://github.com/cucumber/cucumber-jvm/pull/2547) M.P. Korstanje)
24-
2522
## [7.3.4] (2022-05-02)
2623

2724
### Fixed
@@ -1793,16 +1790,17 @@ in `cucumber.api` stable from now on, with proper deprecation warnings in case s
17931790
17941791
<!-- Releases -->
17951792
[Unreleased]: https://github.com/cucumber/cucumber-jvm/compare/v7.3.4...main
1796-
[7.3.4]: https://github.com/cucumber/cucumber-jvm/compare/v7.3.3-RC1...v7.3.4
1797-
[7.3.3]: https://github.com/cucumber/cucumber-jvm/compare/v7.3.2-RC1...v7.3.3
1798-
[7.3.2]: https://github.com/cucumber/cucumber-jvm/compare/v7.3.1-RC1...v7.3.2
1799-
[7.3.1]: https://github.com/cucumber/cucumber-jvm/compare/v7.3.0-RC1...v7.3.1
1800-
[7.3.0]: https://github.com/cucumber/cucumber-jvm/compare/v7.2.3-RC1...v7.3.0
1801-
[7.2.3]: https://github.com/cucumber/cucumber-jvm/compare/v7.2.2-RC1...v7.2.3
1802-
[7.2.2]: https://github.com/cucumber/cucumber-jvm/compare/v7.2.1-RC1...v7.2.2
1803-
[7.2.1]: https://github.com/cucumber/cucumber-jvm/compare/v7.2.0-RC1...v7.2.1
1804-
[7.2.0]: https://github.com/cucumber/cucumber-jvm/compare/v7.1.0-RC1...v7.2.0
1805-
[7.1.0]: https://github.com/cucumber/cucumber-jvm/compare/v7.0.0-RC1...v7.1.0
1793+
[7.3.4]: https://github.com/cucumber/cucumber-jvm/compare/v7.3.3...v7.3.4
1794+
[7.3.4]: https://github.com/cucumber/cucumber-jvm/compare/v7.3.3...v7.3.4
1795+
[7.3.3]: https://github.com/cucumber/cucumber-jvm/compare/v7.3.2...v7.3.3
1796+
[7.3.2]: https://github.com/cucumber/cucumber-jvm/compare/v7.3.1...v7.3.2
1797+
[7.3.1]: https://github.com/cucumber/cucumber-jvm/compare/v7.3.0...v7.3.1
1798+
[7.3.0]: https://github.com/cucumber/cucumber-jvm/compare/v7.2.3...v7.3.0
1799+
[7.2.3]: https://github.com/cucumber/cucumber-jvm/compare/v7.2.2...v7.2.3
1800+
[7.2.2]: https://github.com/cucumber/cucumber-jvm/compare/v7.2.1...v7.2.2
1801+
[7.2.1]: https://github.com/cucumber/cucumber-jvm/compare/v7.2.0...v7.2.1
1802+
[7.2.0]: https://github.com/cucumber/cucumber-jvm/compare/v7.1.0...v7.2.0
1803+
[7.1.0]: https://github.com/cucumber/cucumber-jvm/compare/v7.0.0...v7.1.0
18061804
[7.0.0]: https://github.com/cucumber/cucumber-jvm/compare/v7.0.0-RC1...v7.0.0
18071805
[7.0.0-RC1]: https://github.com/cucumber/cucumber-jvm/compare/v6.11.0...v7.0.0-RC1
18081806
[6.11.0]: https://github.com/cucumber/cucumber-jvm/compare/v6.10.4...v6.11.0

‎spring/src/main/java/io/cucumber/spring/CucumberTestContext.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,14 @@ void registerDestructionCallback(String name, Runnable callback) {
6666
}
6767

6868
void requireActiveScenario() {
69-
if (sessionId == null) {
69+
if (!isActive()) {
7070
throw new IllegalStateException(
7171
"Scenario scoped beans can only be created while Cucumber is executing a scenario");
7272
}
7373
}
7474

75+
boolean isActive() {
76+
return sessionId != null;
77+
}
78+
7579
}

‎spring/src/main/java/io/cucumber/spring/TestContextAdaptor.java

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,10 @@ public final void start() {
5050

5151
private void notifyTestContextManagerAboutBeforeTestMethod() {
5252
try {
53-
Class<?> testClass = delegate.getTestContext().getTestClass();
54-
Object testContextInstance = applicationContext.getBean(testClass);
53+
Class<?> delegateTestClass = delegate.getTestContext().getTestClass();
54+
Object delegateTestInstance = applicationContext.getBean(delegateTestClass);
5555
Method dummyMethod = TestContextAdaptor.class.getMethod("cucumberDoesNotHaveASingleTestMethod");
56-
delegate.beforeTestMethod(testContextInstance, dummyMethod);
56+
delegate.beforeTestMethod(delegateTestInstance, dummyMethod);
5757
} catch (Exception e) {
5858
throw new CucumberBackendException(e.getMessage(), e);
5959
}
@@ -101,8 +101,14 @@ private void registerStepClassBeanDefinition(BeanDefinitionRegistry registry, Cl
101101
}
102102

103103
public final void stop() {
104-
notifyTestContextManagerAboutAfterTestMethod();
105-
CucumberTestContext.getInstance().stop();
104+
// Don't invoke after test method when before test class was not invoked
105+
// this is implicit in the existence of an active the test context
106+
// session. This is not ideal, but Cucumber only supports 1 set of
107+
// before/after semantics while JUnit and Spring have 2 sets.
108+
if (CucumberTestContext.getInstance().isActive()) {
109+
notifyTestContextManagerAboutAfterTestMethod();
110+
CucumberTestContext.getInstance().stop();
111+
}
106112
notifyTestContextManagerAboutAfterTestClass();
107113
}
108114

@@ -116,10 +122,9 @@ private void notifyTestContextManagerAboutAfterTestClass() {
116122

117123
private void notifyTestContextManagerAboutAfterTestMethod() {
118124
try {
119-
Class<?> testClass = delegate.getTestContext().getTestClass();
120-
Object testContextInstance = applicationContext.getBean(testClass);
125+
Object delegateTestInstance = delegate.getTestContext().getTestInstance();
121126
Method dummyMethod = TestContextAdaptor.class.getMethod("cucumberDoesNotHaveASingleTestMethod");
122-
delegate.afterTestMethod(testContextInstance, dummyMethod, null);
127+
delegate.afterTestMethod(delegateTestInstance, dummyMethod, null);
123128
} catch (Exception e) {
124129
throw new CucumberBackendException(e.getMessage(), e);
125130
}

‎spring/src/test/java/io/cucumber/spring/SpringFactoryTest.java

Lines changed: 92 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,15 @@
1818
import io.cucumber.spring.metaconfig.general.BellyMetaStepDefinitions;
1919
import org.junit.jupiter.api.Test;
2020
import org.junit.jupiter.api.function.Executable;
21+
import org.junit.jupiter.params.ParameterizedTest;
22+
import org.junit.jupiter.params.provider.ValueSource;
2123
import org.springframework.beans.factory.BeanCreationException;
2224
import org.springframework.beans.factory.annotation.Autowired;
2325
import org.springframework.beans.factory.annotation.Value;
2426
import org.springframework.test.context.ContextConfiguration;
27+
import org.springframework.test.context.TestContext;
28+
import org.springframework.test.context.TestExecutionListener;
29+
import org.springframework.test.context.TestExecutionListeners;
2530

2631
import java.util.Optional;
2732

@@ -35,7 +40,6 @@
3540
import static org.junit.jupiter.api.Assertions.assertAll;
3641
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
3742
import static org.junit.jupiter.api.Assertions.assertEquals;
38-
import static org.junit.jupiter.api.Assertions.assertNull;
3943
import static org.junit.jupiter.api.Assertions.assertThrows;
4044
import static org.junit.jupiter.api.Assertions.assertTrue;
4145

@@ -57,10 +61,10 @@ void shouldGiveUsNewStepInstancesForEachScenario() {
5761
factory.stop();
5862

5963
assertAll(
60-
() -> assertThat(o1, is(notNullValue())),
61-
() -> assertThat(o2, is(notNullValue())),
62-
() -> assertThat(o1, is(not(equalTo(o2)))),
63-
() -> assertThat(o2, is(not(equalTo(o1)))));
64+
() -> assertThat(o1, is(notNullValue())),
65+
() -> assertThat(o2, is(notNullValue())),
66+
() -> assertThat(o1, is(not(equalTo(o2)))),
67+
() -> assertThat(o2, is(not(equalTo(o1)))));
6468
}
6569

6670
@Test
@@ -101,10 +105,10 @@ void shouldNeverCreateNewApplicationBeanInstances() {
101105
factory2.stop();
102106

103107
assertAll(
104-
() -> assertThat(o1, is(notNullValue())),
105-
() -> assertThat(o2, is(notNullValue())),
106-
() -> assertThat(o1, is(equalTo(o1))),
107-
() -> assertThat(o2, is(equalTo(o2))));
108+
() -> assertThat(o1, is(notNullValue())),
109+
() -> assertThat(o2, is(notNullValue())),
110+
() -> assertThat(o1, is(equalTo(o1))),
111+
() -> assertThat(o2, is(equalTo(o2))));
108112
}
109113

110114
@Test
@@ -124,10 +128,10 @@ void shouldNeverCreateNewApplicationBeanInstancesUsingMetaConfiguration() {
124128
factory2.stop();
125129

126130
assertAll(
127-
() -> assertThat(o1, is(notNullValue())),
128-
() -> assertThat(o2, is(notNullValue())),
129-
() -> assertThat(o1, is(equalTo(o1))),
130-
() -> assertThat(o2, is(equalTo(o2))));
131+
() -> assertThat(o1, is(notNullValue())),
132+
() -> assertThat(o2, is(notNullValue())),
133+
() -> assertThat(o1, is(equalTo(o1))),
134+
() -> assertThat(o2, is(equalTo(o2))));
131135
}
132136

133137
@Test
@@ -143,10 +147,10 @@ void shouldFindStepDefsCreatedImplicitlyForAutowiring() {
143147
factory1.stop();
144148

145149
assertAll(
146-
() -> assertThat(o1.getThirdStepDef(), is(notNullValue())),
147-
() -> assertThat(o2, is(notNullValue())),
148-
() -> assertThat(o1.getThirdStepDef(), is(equalTo(o2))),
149-
() -> assertThat(o2, is(equalTo(o1.getThirdStepDef()))));
150+
() -> assertThat(o1.getThirdStepDef(), is(notNullValue())),
151+
() -> assertThat(o2, is(notNullValue())),
152+
() -> assertThat(o1.getThirdStepDef(), is(equalTo(o2))),
153+
() -> assertThat(o2, is(equalTo(o1.getThirdStepDef()))));
150154
}
151155

152156
@Test
@@ -162,10 +166,10 @@ void shouldReuseStepDefsCreatedImplicitlyForAutowiring() {
162166
factory1.stop();
163167

164168
assertAll(
165-
() -> assertThat(o1.getThirdStepDef(), is(notNullValue())),
166-
() -> assertThat(o3.getThirdStepDef(), is(notNullValue())),
167-
() -> assertThat(o1.getThirdStepDef(), is(equalTo(o3.getThirdStepDef()))),
168-
() -> assertThat(o3.getThirdStepDef(), is(equalTo(o1.getThirdStepDef()))));
169+
() -> assertThat(o1.getThirdStepDef(), is(notNullValue())),
170+
() -> assertThat(o3.getThirdStepDef(), is(notNullValue())),
171+
() -> assertThat(o1.getThirdStepDef(), is(equalTo(o3.getThirdStepDef()))),
172+
() -> assertThat(o3.getThirdStepDef(), is(equalTo(o1.getThirdStepDef()))));
169173
}
170174

171175
@Test
@@ -209,10 +213,10 @@ void shouldRespectDirtiesContextAnnotationsInStepDefs() {
209213
factory.stop();
210214

211215
assertAll(
212-
() -> assertThat(o1, is(notNullValue())),
213-
() -> assertThat(o2, is(notNullValue())),
214-
() -> assertThat(o1, is(not(equalTo(o2)))),
215-
() -> assertThat(o2, is(not(equalTo(o1)))));
216+
() -> assertThat(o1, is(notNullValue())),
217+
() -> assertThat(o2, is(notNullValue())),
218+
() -> assertThat(o1, is(not(equalTo(o2)))),
219+
() -> assertThat(o2, is(not(equalTo(o1)))));
216220
}
217221

218222
@Test
@@ -232,10 +236,10 @@ void shouldRespectDirtiesContextAnnotationsInStepDefsUsingMetaConfiguration() {
232236
factory.stop();
233237

234238
assertAll(
235-
() -> assertThat(o1, is(notNullValue())),
236-
() -> assertThat(o2, is(notNullValue())),
237-
() -> assertThat(o1, is(not(equalTo(o2)))),
238-
() -> assertThat(o2, is(not(equalTo(o1)))));
239+
() -> assertThat(o1, is(notNullValue())),
240+
() -> assertThat(o2, is(notNullValue())),
241+
() -> assertThat(o1, is(not(equalTo(o2)))),
242+
() -> assertThat(o2, is(not(equalTo(o1)))));
239243
}
240244

241245
@Test
@@ -257,9 +261,9 @@ void shouldFailIfMultipleClassesWithSpringAnnotationsAreFound() {
257261
Executable testMethod = () -> factory.addClass(BellyStepDefinitions.class);
258262
CucumberBackendException actualThrown = assertThrows(CucumberBackendException.class, testMethod);
259263
assertThat(actualThrown.getMessage(), startsWith(
260-
"Glue class class io.cucumber.spring.contextconfig.BellyStepDefinitions and class io.cucumber.spring.SpringFactoryTest$WithSpringAnnotations are both annotated with @CucumberContextConfiguration.\n"
261-
+
262-
"Please ensure only one class configures the spring context"));
264+
"Glue class class io.cucumber.spring.contextconfig.BellyStepDefinitions and class io.cucumber.spring.SpringFactoryTest$WithSpringAnnotations are both annotated with @CucumberContextConfiguration.\n"
265+
+
266+
"Please ensure only one class configures the spring context"));
263267
}
264268

265269
@Test
@@ -269,7 +273,7 @@ void shouldFailIfClassWithSpringComponentAnnotationsIsFound() {
269273
Executable testMethod = () -> factory.addClass(WithComponentAnnotation.class);
270274
CucumberBackendException actualThrown = assertThrows(CucumberBackendException.class, testMethod);
271275
assertThat(actualThrown.getMessage(), is(equalTo(
272-
"Glue class io.cucumber.spring.componentannotation.WithComponentAnnotation was annotated with @Component; marking it as a candidate for auto-detection by Spring. Glue classes are detected and registered by Cucumber. Auto-detection of glue classes by spring may lead to duplicate bean definitions. Please remove the @Component annotation")));
276+
"Glue class io.cucumber.spring.componentannotation.WithComponentAnnotation was annotated with @Component; marking it as a candidate for auto-detection by Spring. Glue classes are detected and registered by Cucumber. Auto-detection of glue classes by spring may lead to duplicate bean definitions. Please remove the @Component annotation")));
273277
}
274278

275279
@Test
@@ -279,7 +283,7 @@ void shouldFailIfClassWithAnnotationAnnotatedWithSpringComponentAnnotationsIsFou
279283
Executable testMethod = () -> factory.addClass(WithControllerAnnotation.class);
280284
CucumberBackendException actualThrown = assertThrows(CucumberBackendException.class, testMethod);
281285
assertThat(actualThrown.getMessage(), is(equalTo(
282-
"Glue class io.cucumber.spring.componentannotation.WithControllerAnnotation was annotated with @Controller; marking it as a candidate for auto-detection by Spring. Glue classes are detected and registered by Cucumber. Auto-detection of glue classes by spring may lead to duplicate bean definitions. Please remove the @Controller annotation")));
286+
"Glue class io.cucumber.spring.componentannotation.WithControllerAnnotation was annotated with @Controller; marking it as a candidate for auto-detection by Spring. Glue classes are detected and registered by Cucumber. Auto-detection of glue classes by spring may lead to duplicate bean definitions. Please remove the @Controller annotation")));
283287
}
284288

285289
@Test
@@ -300,10 +304,10 @@ void shouldGlueScopedSpringBeanBehaveLikeGlueLifecycle() {
300304
factory.stop();
301305

302306
assertAll(
303-
() -> assertThat(glueInstance1, is(not(glueInstance2))),
304-
() -> assertThat(glueInstance2, is(not(glueInstance1))),
305-
() -> assertThat(bellyInstance1, is(bellyInstance2)),
306-
() -> assertThat(bellyInstance2, is(bellyInstance1)));
307+
() -> assertThat(glueInstance1, is(not(glueInstance2))),
308+
() -> assertThat(glueInstance2, is(not(glueInstance1))),
309+
() -> assertThat(bellyInstance1, is(bellyInstance2)),
310+
() -> assertThat(bellyInstance2, is(bellyInstance1)));
307311
}
308312

309313
@Test
@@ -327,7 +331,7 @@ void shouldBeStoppableWhenFacedWithInvalidConfiguration() {
327331

328332
IllegalStateException exception = assertThrows(IllegalStateException.class, factory::start);
329333
assertThat(exception.getMessage(),
330-
containsString("DelegatingSmartContextLoader was unable to detect defaults"));
334+
containsString("DelegatingSmartContextLoader was unable to detect defaults"));
331335
assertDoesNotThrow(factory::stop);
332336
}
333337

@@ -341,6 +345,19 @@ void shouldBeStoppableWhenFacedWithMissingContextConfiguration() {
341345
assertDoesNotThrow(factory::stop);
342346
}
343347

348+
@ParameterizedTest
349+
@ValueSource(classes = {
350+
FailedBeforeTestClassContextConfiguration.class,
351+
FailedBeforeTestMethodContextConfiguration.class,
352+
})
353+
void shouldBeStoppableWhenFacedWithFailedApplicationContext(Class<?> contextConfiguration) {
354+
final ObjectFactory factory = new SpringFactory();
355+
factory.addClass(contextConfiguration);
356+
357+
assertThrows(CucumberBackendException.class, factory::start);
358+
assertDoesNotThrow(factory::stop);
359+
}
360+
344361
@CucumberContextConfiguration
345362
@ContextConfiguration("classpath:cucumber.xml")
346363
public static class WithSpringAnnotations {
@@ -366,7 +383,7 @@ public String getProperty() {
366383
}
367384

368385
@CucumberContextConfiguration
369-
@ContextConfiguration()
386+
@ContextConfiguration
370387
public static class WithEmptySpringAnnotations {
371388

372389
}
@@ -376,4 +393,39 @@ public static class WithoutContextConfiguration {
376393

377394
}
378395

396+
@CucumberContextConfiguration
397+
@ContextConfiguration("classpath:cucumber.xml")
398+
@TestExecutionListeners(FailedBeforeTestClassContextConfiguration.FailingListener.class)
399+
public static class FailedBeforeTestClassContextConfiguration {
400+
401+
public static class FailingListener implements TestExecutionListener {
402+
403+
@Override
404+
public void beforeTestClass(TestContext testContext) throws Exception {
405+
throw new StubException();
406+
}
407+
408+
}
409+
410+
}
411+
412+
@CucumberContextConfiguration
413+
@ContextConfiguration("classpath:cucumber.xml")
414+
@TestExecutionListeners(FailedBeforeTestMethodContextConfiguration.FailingListener.class)
415+
public static class FailedBeforeTestMethodContextConfiguration {
416+
417+
public static class FailingListener implements TestExecutionListener {
418+
419+
@Override
420+
public void beforeTestMethod(TestContext testContext) throws Exception {
421+
throw new StubException();
422+
}
423+
424+
}
425+
426+
}
427+
428+
public static class StubException extends Exception {
429+
430+
}
379431
}

0 commit comments

Comments
 (0)