Using @ActiveProfiles with @SpringBootTest now adds to the profiles configured using spring.profiles.active rather than overriding them

[achatellier]

Hello, I have active profile issues with Spring Boot 2.2.3.RELEASE.

Step to reproduce :

• git clone https://github.com/spring-projects/spring-petclinic
• add spring.profiles.active=prod to application.properties
• add @ExtendWith(SpringExtension.class) and @ActiveProfiles("test") to PetclinicIntegrationTests
• Run the test PetclinicIntegrationTests

You can see: The following profiles are active: test diplayed in logs

Now change spring-boot-starter-parent to 2.2.3.RELEASE and re-run the test.

You can see: The following profiles are active: test,prod diplayed in logs

[wilkinsona]

Thanks for the report. I suspect that this is a side-effect of the changes for #19556.

[wilkinsona]

#19556 is the cause. Prior to that change, @ActiveProfiles("test") was mapped onto adding a property source to the environment that sets spring.profiles.active. This overrode the property in application.properties. With the change for #19556 in place, @ActiveProfiles("test") now maps onto a call to Environment.setActiveProfiles("test"). This leaves the value of spring.profiles.active in application.properties to be processed by ConfigFileApplicationListener which results in a call to addActiveProfile("prod"). As a result, both the test and prod profiles are active.

You can get 2.2.2's behaviour in 2.2.3 by removing @ActiveProfiles("test") and configuring spring.profiles.active instead:

@SpringBootTest(properties = "spring.profiles.active:test")

In some ways, I think 2.2.3's behaviour is better. It makes it possible to add an active profile while also making it possible to override the active profiles as well. In 2.2.2, only overriding entirely was possible. However, the change in behaviour in a maintenance release isn't ideal. Flagging for team attention so that we can consider if we want to restore the old behaviour in 2.2.4 which would require finding a different solution for #19556.

[wilkinsona]

I should also have mentioned that using @ExtendWith(SpringExtension.class) isn't important here and it isn't needed. @SpringBootTest is already annotated with @ExtendWith(SpringExtension.class) so you can remove it from test classes annotated with @SpringBootTest with no change in behaviour.

[dogilvie]

We have seen breakage in our tests which could be related to this. The @ActiveProfiles are not seen in bean creation so that the context fails to load. This unit test below works in 2.2.2 but fails in 2.2.3

import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.context.ActiveProfiles;

@SpringBootTest
@ActiveProfiles("junit")
public class SimpleTest {

	@Value("${spring.profiles.active}")
	String activeProfiles;
	
	@Test
	public void doit()
	{
		assertEquals("junit", activeProfiles,"Should be junit");
	}
	
}

java.lang.IllegalArgumentException: Could not resolve placeholder 'spring.profiles.active' in value "${spring.profiles.active}"

[wilkinsona]

@dogilvie Thank you. That is the same problem. It may well be addressed by this issue, but I would encourage you not to rely upon it. You should consider the use of the spring.profiles.active property to be an implementation detail of @ActiveProfiles. If you have code that relies upon that property being set, I would recommend double-checking that it is really necessary to do so. If it is, I’d recommend setting the property directly, for example by using the properties attribute on @SpringBootTest

[wilkinsona]

I wondered if we could fix this without breaking the fix for #19556 by setting spring.profiles.active[n] for each profile in the array:

diff --git a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootContextLoader.java b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootContextLoader.java
index 99fb6a5f5b..71350fbe12 100644
--- a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootContextLoader.java
+++ b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/SpringBootContextLoader.java
@@ -29,6 +29,7 @@ import org.springframework.boot.context.properties.source.ConfigurationPropertyS
 import org.springframework.boot.context.properties.source.MapConfigurationPropertySource;
 import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;
 import org.springframework.boot.test.mock.web.SpringBootMockServletContext;
+import org.springframework.boot.test.util.TestPropertyValues;
 import org.springframework.boot.web.reactive.context.GenericReactiveWebApplicationContext;
 import org.springframework.boot.web.servlet.support.ServletContextApplicationContextInitializer;
 import org.springframework.context.ApplicationContext;
@@ -97,7 +98,7 @@ public class SpringBootContextLoader extends AbstractContextLoader {
                application.getSources().addAll(Arrays.asList(configLocations));
                ConfigurableEnvironment environment = getEnvironment();
                if (!ObjectUtils.isEmpty(config.getActiveProfiles())) {
-                       environment.setActiveProfiles(config.getActiveProfiles());
+                       setActiveProfiles(environment, config.getActiveProfiles());
                }
                ResourceLoader resourceLoader = (application.getResourceLoader() != null) ? application.getResourceLoader()
                                : new DefaultResourceLoader(getClass().getClassLoader());
@@ -125,6 +126,14 @@ public class SpringBootContextLoader extends AbstractContextLoader {
                return application.run(getArgs(config));
        }
 
+       private void setActiveProfiles(ConfigurableEnvironment environment, String[] profiles) {
+               List<String> pairs = new ArrayList<>();
+               for (int i = 0; i < profiles.length; i++) {
+                       pairs.add("spring.profiles.active[" + i + "]=" + profiles[i]);
+               }
+               TestPropertyValues.of(pairs).applyTo(environment);
+       }
+
        /**
         * Builds new {@link org.springframework.boot.SpringApplication} instance. You can
         * override this method to add custom behavior

It works in terms of SpringBootContextLoaderTests but reveals a limitation in ConfigFileApplicationListener where it looks for the spring.profiles.active property. This doesn't work when profiles have been set using indexes. For example, this means that launching an app with -Dspring.profiles.active[0]=a,b doesn't activate a profile named a,b (the default profile is used) and launching it with -Dspring.profiles.active=a,b activates profiles a and b.

In short, there are other areas where profiles with commas in their names do not work as might be expected. #19537 was a hypothetical problem so I think we should revert the changes made in #19556 and re-open #19537 to see if it's something that we want to try and support in the future.

[dogilvie]

You should consider the use of the spring.profiles.active property to be an implementation detail of @ActiveProfiles.

@wilkinsona can you explain further why you take this view? It seemed to me that the property and the annotation are both independent mechanisms for writing the "spring active profiles". And the property had the additional function of being able to read back the "spring active profiles" value.

I would suggest a new @AddActiveProfiles annotation if you want increased flexibility.

[wilkinsona]

Largely because it isn't documented anywhere. The purpose of @ActiveProfiles is to make some profiles active. Exactly how that is achieved isn't specified and, if at all possible, shouldn't be relied upon.

Application code that depends upon the spring.profiles.active property is rather unusual. As I said above, I would consider revisiting that code and seeing if it can be avoided. Injecting the Environment and calling getActiveProfiles() may well be more robust. If you really do need the spring.profiles.active property to be set, I'd recommend setting it rather than relying on a side-effect of something else setting it for you.

[tmysik]

@wilkinsona

Application code that depends upon the spring.profiles.active property is rather unusual.

Let me ask - what is the best way to run tests with the test profile? It is usually needed in order to load the proper application properties (test.properties). Or are you talking about the application itself and not about its tests? Thank you.

[wilkinsona]

When I said "depends upon" above, I really meant consumes. I wouldn't expect test or main code to consume spring.profiles.active. Either may set it for a variety of reasons. In terms of test code, testing with a particular profile active is the most obvious one. In terms of main code, it's less likely but you may want to set a default spring.profiles.active property in the main method for example.