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

Add the ability to specify active profiles in @SpringJUnitConfig [SPR-17101] #21638

Closed
spring-projects-issues opened this issue Jul 27, 2018 · 10 comments

Comments

@spring-projects-issues
Copy link
Collaborator

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

Alexey Trofimov opened SPR-17101 and commented

It would be cool if you could set the profile for testing right in this annotation.

Now, i must write 2 annotations in every test:

@SpringJUnitConfig
@ActiveProfiles("test")

Want write something like this:

@SpringJUnitConfig(profiles = "test")

I think, most JUnit tests uses custom test profile.

Why not using system property? Because if you develop with IDE, for example, IntelliJ IDEA, you can run test by one click and IDEA must know, which profile must be used for this test.


No further details from SPR-17101

@spring-projects-issues
Copy link
Collaborator Author

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

Sam Brannen commented

For what it's worth, it's actually quite easy to create your own custom composed annotation to achieve this.

For example:

@ExtendWith(SpringExtension.class)
@ContextConfiguration
@ActiveProfiles
@Documented
@Inherited
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
public @interface MySpringJUnitConfig {

	@AliasFor(annotation = ContextConfiguration.class, attribute = "classes")
	Class<?>[] value() default {};

	@AliasFor(annotation = ContextConfiguration.class)
	Class<?>[] classes() default {};

	@AliasFor(annotation = ContextConfiguration.class)
	String[] locations() default {};

	@AliasFor(annotation = ContextConfiguration.class)
	Class<? extends ApplicationContextInitializer<?>>[] initializers() default {};

	@AliasFor(annotation = ContextConfiguration.class)
	boolean inheritLocations() default true;

	@AliasFor(annotation = ContextConfiguration.class)
	boolean inheritInitializers() default true;

	@AliasFor(annotation = ContextConfiguration.class)
	String name() default "";

	@AliasFor(annotation = ActiveProfiles.class)
	String[] profiles() default {};

	@AliasFor(annotation = ActiveProfiles.class, value = "resolver")
	Class<? extends ActiveProfilesResolver> profilesResolver() default ActiveProfilesResolver.class;

	@AliasFor(annotation = ActiveProfiles.class)
	boolean inheritProfiles() default true;

}

Loading

@spring-projects-issues
Copy link
Collaborator Author

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

Sam Brannen commented

I think, most JUnit tests uses custom test profile.

I'm not convinced that we can safely make that assumption.

In any case, I am a bit hesitant to introduce support for active profiles directly in @SpringJUnitConfig.

As you can see from my example @MySpringJUnitConfig, doing so properly already requires the addition of three annotation attributes.

If we were to support active profiles directly in @SpringJUnitConfig, additional developers might come along and request the same level of support for @TestPropertySource, @DirtiesContext, @Commit/@Rollback, etc.

So, we could essentially be opening up a can of worms, and I would like to avoid going down that road.

Loading

@spring-projects-issues
Copy link
Collaborator Author

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

Alexey Trofimov commented

Yeah, of course i can write custom compose annotation for this. Maybe I misunderstood something, but for what purpose is this annotation?

Now, for typical spring junit tests i must write set of annotations:

@ExtendWith(SpringExtension.class)
@ActiveProfiles("test")
@DataJpaTest // or @SpringBootTest or @JdbcTest or etc. 

 Why should i use @``SpringJUnitConfig annotation? Only for replace @ExtendWith annotation? Because, if i want to use that annotation, i must write:

@SpringJUnitConfig
@ActiveProfiles("test")
@DataJpaTest // or @SpringBootTest or @JdbcTest or etc. 

About your comment: I probably exaggerated, but there are categories of tests that usually can not be run without a custom profile, for example: @``SpringBootTest, @DataJpa``Test, @Jdbc``Test and other. And I see 2 variants for simplifying the current testing process:

  1. set the profile for the tests from the annotation @SpringJUnitConfig (because, i need set this annotation with every test, where i'm using spring context)
  2. set the profile from the annotations, such as @SpringBootTest, @DataJpaTest, @JdbcTest

I think, second variant, more difficult, than first.

PS Thanks for the greate JUnit Jupiter library - today it's a most powerfull and perspective test tool.

Loading

@spring-projects-issues
Copy link
Collaborator Author

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

Sam Brannen commented

for what purpose is this annotation?

As stated in the class-level JavaDoc for @SpringJUnitConfig:

@SpringJUnitConfig is a composed annotation that combines @ExtendWith(SpringExtension.class) from JUnit Jupiter with @ContextConfiguration from the Spring TestContext Framework.

Why should i use @SpringJUnitConfig annotation? Only for replace @ExtendWith annotation?

No. It serves two purposes.

  1. It's semantically clearer than using @ExtendWith(SpringExtension.class) directly.
  2. It simplifies matters if you wish to register the SpringExtension and declare configuration classes, etc. (that you would otherwise do via @ContextConfiguration).

Loading

@spring-projects-issues
Copy link
Collaborator Author

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

Sam Brannen commented

Please keep in mind that @SpringBootTest, @DataJpaTest, and @JdbcTest are not part of Core Spring. They are test annotations from Spring Boot.

Thus, the Core Spring team cannot add attributes to annotations such as @SpringBootTest, @DataJpaTest, and @JdbcTest.

If you are using Spring Boot, you typically would not use @ContextConfiguration directly anyway. Instead, you would rely on Spring Boot's support for automatically locating your @SpringBootApplication class, in which case you might find @ExtendWith(SpringExtension.class) more suitable than @SpringJUnitConfig.

Loading

@spring-projects-issues
Copy link
Collaborator Author

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

Sam Brannen commented

PS Thanks for the greate JUnit Jupiter library - today it's a most powerfull and perspective test tool.

You're very welcome, and I'm pleased to hear that you like it! :)

Loading

@spring-projects-issues
Copy link
Collaborator Author

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

Sam Brannen commented

Another thing I'd like to point out is that Spring Boot's testing support often does not encourage users to declare @ContextConfiguration.

Thus, I'm not certain that is a good idea to recommend users to use @SpringJUnitConfig in conjunction with Spring Boot's test annotations.

Phil Webb, what are your thoughts on this matter?

Loading

@spring-projects-issues
Copy link
Collaborator Author

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

Alexey Trofimov commented

Sam Brannen thanks for the answer. Ok, I'm understood.Trying to ask memebers of the spring-boot project, maybe they can add profiles-property to theirs annotations

Loading

@spring-projects-issues
Copy link
Collaborator Author

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

Phil Webb commented

I honestly prefer the separate @ActiveProfiles annotation I think. Adding another attribute to all the existing annotations seems a little invasive.

Loading

@spring-projects-issues
Copy link
Collaborator Author

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

Sam Brannen commented

Thanks for the feedback, Alexey Trofimov and Phil Webb.

 In light of that, I am closing this issue.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants