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

Relax explicit TestContextBootstrapper resolution [SPR-17006] #21544

Closed
3 of 4 tasks
spring-projects-issues opened this issue Jul 4, 2018 · 7 comments
Closed
3 of 4 tasks
Assignees
Labels
in: test type: enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Jul 4, 2018

Sam Brannen opened SPR-17006 and commented

Status Quo

The resolution process for explicit TestContextBootstrapper registration via @BootstrapWith currently throws an IllegalStateException if more than one declaration of @BootstrapWith is detected.

As mentioned in Moduliths Issue #5, the resolution can be relaxed by checking the concrete types registered via @BootstrapWith.

Proposals

  1. If multiple @BootstrapWith declarations are detected but each register the exact same type of boostrapper, no exception should occur.
  2. If multiple @BootstrapWith declarations are detected and all types registered exist within the same class hierarchy, no exception should occur, and the most specific type should be used.
  3. Otherwise, throw an IllegalStateException.
  4. If @BootstrapWith is declared locally, directly on a test class, that declaration should override any such declarations that are meta-present.

Deliverables

  1. Proposal #1 already works.
  2. Consider implementing proposal #2.
  3. Implement proposal #4 (directly present annotation overrides meta-present annotations).
  4. Ensure that an exception is still thrown otherwise.

Affects: 4.3.10

Reference URL: moduliths/moduliths#5

Issue Links:

  • #17203 Explicit failure if multiple @BootstrapWith annotations are used on the same test
@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 4, 2018

Sam Brannen commented

Regarding proposal #1, that appears to already be the case.

 
See comments here.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 6, 2018

Sam Brannen commented

This is related to #17203.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 6, 2018

Sam Brannen commented

FYI: I added a "directly present annotation overrides meta-present annotations" deliverable based on discussions in Moduliths Issue #5.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 15, 2018

Sam Brannen commented

This has been addressed in commit d20d95b as follows:

Allow directly present @BootstrapWith to override meta annotations

Prior to this commit, if a test class was meta-annotated with multiple @BootstrapWith declarations that registered different TestContextBootstrapper implementations, such a configuration would result in an IllegalStateException, and there was no way to override this behavior.

This commit addresses this shortcoming by relaxing the explicit TestContextBootstrapper resolution in BootstrapUtils so that a directly present @BootstrapWith annotation will now override declarations of @BootstrapWith that are meta-present. In other words, if @BootstrapWith is used as a meta-annotation, it can be overridden directly on the test class via an explicit, local declaration of @BootstrapWith.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 15, 2018

Sam Brannen commented

Oliver Drotbohm, I figured you'd like to know this has been addressed for Spring 5.1.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 15, 2018

Oliver Drotbohm commented

Thanks for the heads up, Sam Brannen. Looks useful to me :).

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 15, 2018

Sam Brannen commented

You're welcome, Oliver Drotbohm!

Phil Webb, you may also find this useful to know since you originally requested the previous behavior.

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

No branches or pull requests

2 participants