Skip to content

Document that test slices should not be combined #14981

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

Closed
G-Lem opened this issue Oct 27, 2018 · 11 comments
Closed

Document that test slices should not be combined #14981

G-Lem opened this issue Oct 27, 2018 · 11 comments
Labels
status: superseded An issue that has been superseded by another

Comments

@G-Lem
Copy link

G-Lem commented Oct 27, 2018

Could you have a look at this issue ?
https://stackoverflow.com/questions/52527394/in-spring-boot-2-1-many-test-slices-are-not-allowed-anymore-due-to-multiple-boo?noredirect=1#

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

To summarize the stackoverflow question, the following test:

@RunWith(SpringRunner.class)
@JdbcTest
@JsonTest
public class MyTest {

	@Test
	public void test() {
		System.out.println("Hello, World !");
	}

}

Fails with Spring Boot 2.1 due to:

java.lang.IllegalStateException: Configuration error: found multiple declarations of @BootstrapWith for test class [scratch.MyTest]: [@org.springframework.test.context.BootstrapWith(value=class org.springframework.boot.test.autoconfigure.jdbc.JdbcTestContextBootstrapper), @org.springframework.test.context.BootstrapWith(value=class org.springframework.boot.test.autoconfigure.json.JsonTestContextBootstrapper)]
	at org.springframework.test.context.BootstrapUtils.resolveExplicitTestContextBootstrapper(BootstrapUtils.java:166)
	at org.springframework.test.context.BootstrapUtils.resolveTestContextBootstrapper(BootstrapUtils.java:127)
	at org.springframework.test.context.TestContextManager.<init>(TestContextManager.java:124)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.createTestContextManager(SpringJUnit4ClassRunner.java:151)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.<init>(SpringJUnit4ClassRunner.java:142)
	at org.springframework.test.context.junit4.SpringRunner.<init>(SpringRunner.java:49)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
	at org.junit.internal.builders.AnnotatedBuilder.buildRunner(AnnotatedBuilder.java:104)
	at org.junit.internal.builders.AnnotatedBuilder.runnerForClass(AnnotatedBuilder.java:86)
	at org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:59)
	at org.junit.internal.builders.AllDefaultPossibilitiesBuilder.runnerForClass(AllDefaultPossibilitiesBuilder.java:26)
	at org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:59)
	at org.junit.internal.requests.ClassRequest.getRunner(ClassRequest.java:33)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestLoader.createUnfilteredTest(JUnit4TestLoader.java:87)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestLoader.createTest(JUnit4TestLoader.java:73)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestLoader.loadTests(JUnit4TestLoader.java:46)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:522)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:760)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:460)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:206)

@philwebb
Copy link
Member

The root cause is the change made for #14052 which means we now have two different bootstrap classes.

@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 28, 2018
@philwebb philwebb added this to the 2.1.x milestone Oct 28, 2018
@philwebb
Copy link
Member

I wonder if we can meta-annotate @JdbcTest with @SpringBootTest and use @AliasFor to get the properties.

@dreis2211
Copy link
Contributor

@philwebb if you don't mind I'd like to give this a shot.

@dreis2211
Copy link
Contributor

dreis2211 commented Oct 28, 2018

Your idea works, but not for @WebMvcTest and @WebFluxTest because they already have dedicated bootstrap classes. Any thoughts? E.g. fixing it for the others with your idea and document the shortcomings of the other two?
In fact they already do that somehow (as well the others):

Can be used when a test focuses **only** on...

I wonder if multiple slices are supported at all officially.

@philwebb
Copy link
Member

I wonder if multiple slices are supported at all officially.

@dreis2211 Thinking about this some more, I'm of the opinion that it worked by luck last time and it isn't something we should officially support. It's going to add a lot of complexity if we let people compose these annotations. For example, how would be combine the new properties attributes.

I'll switch this one to a documentation issue.

@philwebb philwebb added type: documentation A documentation update and removed type: bug A general bug labels Oct 28, 2018
@philwebb
Copy link
Member

@G-Lem I just posted an answer to the stackoverflow question https://stackoverflow.com/a/53033018/1526192

@dreis2211
Copy link
Contributor

@philwebb For learning purposes I made a best effort implementation that seems to be working. If you're interested: master...dreis2211:gh-14981

This also already includes a change in the documentation, which I'm happy to isolate in a dedicated PR if necessary.

Let me know what you think,

@philwebb philwebb changed the title Many test slices are not possible anymore with spring boot 2.1 Document that test slices should not be combined Oct 28, 2018
@wilkinsona
Copy link
Member

wilkinsona commented Nov 27, 2018

@dreis2211 A pull request of that documentation change would be great, please. Perhaps reworded slightly to make it sounds more definite by removing "officially" and replacing "you can pick" with "pick"?

@dreis2211
Copy link
Contributor

@wilkinsona Give me a minute or two

@wilkinsona
Copy link
Member

wilkinsona commented Nov 27, 2018

Wow, that was fast! Thank you, @dreis2211. Closing in favour of #15310.

@wilkinsona wilkinsona added status: superseded An issue that has been superseded by another and removed type: documentation A documentation update labels Nov 27, 2018
@wilkinsona wilkinsona removed this from the 2.1.x milestone Nov 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
Development

No branches or pull requests

5 participants