springboot Make use of @DynamicPropertySource in our integration tests

There's a bit of ceremony involved at the moment:

@Testcontainers
@ContextConfiguration(initializers = DataRedisTestIntegrationTests.Initializer.class)
@DataRedisTest
public class DataRedisTestIntegrationTests {

    @Container
    public static RedisContainer redis = new RedisContainer();

    // …

    static class Initializer
            implements ApplicationContextInitializer<ConfigurableApplicationContext> {

        @Override
        public void initialize(
                ConfigurableApplicationContext configurableApplicationContext) {
            TestPropertyValues.of("spring.redis.port=" + redis.getMappedPort())
                    .applyTo(configurableApplicationContext.getEnvironment());
        }

    }

It would be nice to be able to do something like this instead:

@Testcontainers
@DataRedisTest
public class DataRedisTestIntegrationTests {

    @Container
    @MapPortToProperty("spring.redis.port")
    public static RedisContainer redis = new RedisContainer();

A few thoughts:

• The examples above are JUnit 5. We'd need to consider JUnit 4 as well
• The lifecycle ordering will need to be right so that the container's port is available before the context refreshes
• Setting the port will affect context caching

/cc @bsideup

Comment From: bsideup

@wilkinsona Take a look at https://github.com/Playtika/testcontainers-spring-boot

Also, there are a few things to consider covering:

• it is very important to also map the host, not just port. It is "localhost" only in ~60% cases
• some containers provide convenient methods like KafkaContainer#getBootstrapServers, perhaps we can use a SpEL expression or something

BTW I have a bit crazy idea: What if Spring Boot implements something like:

@Testcontainers
@DataRedisTest
public class DataRedisTestIntegrationTests {

    @Container
    public static RedisContainer redis = new RedisContainer();

    @PropertyContributor
    static Callable<PropertySource> propertySource = () -> {
        return TestPropertyValues.of(
            "spring.redis.host=" + redis.getContainerIpAddress(),
            "spring.redis.port=" + redis.getMappedPort()
        );
    }

This way it can source properties from anywhere, including Testcontainers. WDYT?

Comment From: antkorwin

Hi, I think I solved this in my library (here: https://github.com/jupiter-tools/spring-test-redis), maybe it can help you:

@SpringBootTest
@RedisTestContainer
class RedisTestContainerTest {

    @Autowired
    private RedisTemplate redisTemplate;

    @Test
    void readWriteValueByRedisTemplate() {
        String key = "test";
        String value = "sabracadabra";
        // Act
        redisTemplate.opsForValue().set(key, value);
        // Assert
        assertThat(redisTemplate.opsForValue().get(key)).isEqualTo(value);
    }
}

this code run Redis test container and bind it to default spring boot properties (spring.redis.host / spring.redis.port)

I use my own ContextCustomizer, it provides an ability to start multiple containers and avoids problems with context caching in tests. Also, it provides independence on the test engine (you can use JUnit4 and JUnit5)

Example of executing multiple containers in the one test case:

@SpringBootTest
@RedisTestContainer 
@RedisTestContainer(hostTargetProperty = "second.redis.host", portTargetProperty = "second.redis.port")
@RedisTestContainer(hostTargetProperty = "third.redis.host", portTargetProperty = "third.redis.port") 
class MultipleContainerInOneTest {
    ...
}

the second container set host/port value in the specified properties, after start application context.

Maybe we can use it as part of the spring boot? I am ready to make a PR here.

Comment From: wilkinsona

Thanks for the offer, @antkorwin, but that's not quite what we're looking for here. The use of Redis was just an example and my goal is to provide something that works for any container that's being managed by Testcontainers. That would be something along the lines of the general purpose annotation in my opening description or the property contributor imagined by @bsideup.

Comment From: nosan

I have a slightly different approach. Please take a look https://github.com/nosan/spring-boot/commits/map-to-property This branch introduces @MapToProperty annotation. This annotation can be applied only to static fields.

Here is a quick example of how this annotation can be used:

class DataRedisTests {


// (2) Suppliers    
    @Container
    static final RedisContainer redis = new RedisContainer();
    @MapToProperty("spring.redis.host")
    static final Supplier<String> host  = redis::getContainerIpAddress;
    @MapToProperty("spring.redis.port")
    static final Supplier<Integer> REDIS_PORT_SUPPLIER = redis::getFirstMappedPort;

// (3) Constants    
    @MapToProperty( "spring.redis.port")
    static final int port = 6379;
// (4) Map
    @MapToProperty("spring.redis")
    static final Supplier<Map<String, Object>> properties = () -> {
        Map<String, Object> source = new LinkedHashMap<>();
        source.put("host", redis.getContainerIpAddress());
        source.put("port", redis.getFirstMappedPort());
        return source;
    };
}

Please let me know what you think.

Comment From: philwebb

The low level property mapping support is quite nice, but I can't help feeling that we should be able to apply these conventions automatically. I wonder if we can provide some pre-supplied mapping conventions that detect well known containers and map them to well known Spring Boot properties.

In other words, can we make it so that this lone will get you spring.redis.host and spring.redis.port properties:

@Testcontainers
@DataRedisTest
public class DataRedisTestIntegrationTests {

    @Container
    public static RedisContainer redis = new RedisContainer();

Comment From: wilkinsona

That feels a little bit too magic to me.

Comment From: bsideup

@philwebb as much as I like the idea of having a native support for Testcontainers in Spring Boot Test, I agree with Andy that it is too magical, plus, since only a sub-set of containers will be supported, it can be confusing when somebody will declare a container (especially the generic one) an expect Spring Boot to do the magic without realizing that it is on per-type basis. PropertyContributor looks like a good middle ground here.

@nosan

@MapToProperty("spring.redis") static final Supplier<Map<String, Object>> properties = () -> {

While I understand why you want to do this, most probably you will have more than one container, and the syntactic overhead of defining a few property contributors will overcome the benefit

Comment From: nosan

@bsideup

       @MapToProperty
    static final Supplier<Map<String, Object>> properties = () -> {
        Map<String, Object> source = new LinkedHashMap<>();
        source.put("spring.redis.port", redis.getFirstMappedPort());
        source.put("spring.redis.host", redis.getContainerIpAddress());
        source.put("spring.data.neo4j.uri", neo4j.getBoltUrl());
        return source;
    };

value attribute can be omitted for a Map. From my perspective with @PropertyContributor a bit difficult to define a simple property.

Comment From: nosan

@philwebb It would be nice to support not only TestContainers. I have a project embedded-cassandra and it has CassandraRule class which can be used in conjunction with @ClassRule to start and stop Cassandra. So with @PropertyContributor or @MapToProperty I would be able to do:

    @ClassRule
    public static final CassandraRule cassandra = new CassandraRule();

    @MapToProperty("spring.data.cassandra")
    public static final Supplier<Map<String, Object>> cassandraProperties = () -> {
        Map<String, Object> properties = new LinkedHashMap<>();
        Settings settings = cassandra.getSettings();
        properties.put("port", settings.getPort());
        properties.put("contact-points", settings.getAddress().getHostAddress());
        return properties;
    };

Comment From: nosan

I've updated my approach a bit in my branch: https://github.com/nosan/spring-boot/commits/map-to-property

Here is a list of types that can be used with @MapToProperty:

//spring=boot
@MapToProperty("spring")
String name = "boot";

//spring[0]=boot, spring[1]=framework
@MapToProperty("spring")
Collection<String> collection =Arrays.asList("boot","framework");

//spring[0]=boot, spring[1]=framework
@MapToProperty("spring")
String[]array={"boot","framework"};

//spring=boot
@MapToProperty("spring")
Callable<String> callable=()->"boot";

//spring-boot.version=2.0.0
@MapToProperty("spring-boot")
Map<String, Object> prefixMap=Collections.singletonMap("version","2.0.0");

//spring-framework.version=5.0.0
@MapToProperty
Map<String, Object> map=Collections.singletonMap("spring-framework.version","5.0.0");

//spring=boot
@MapToProperty("spring")
Optional<String> optional=Optional.of("boot");

//spring=boot
@MapToProperty("spring")
Supplier<String> supplier=()->"boot";

//spring-boot.version=2.0.0
@MapToProperty("spring-boot")
TestPropertyValues prefixValues=TestPropertyValues.of("version=2.0.0");

//spring-framework.version=5.0.0
@MapToProperty
TestPropertyValues values=TestPropertyValues.of("spring-framework.version=5.0.0");

//spring.redis.host=redis.getContainerIpAddress()
//spring.redis.port=redis.getMappedPort()
@MapToRedisProperties
 Supplier<TestPropertyValues> propertySource = () -> TestPropertyValues.of(
        "host=" + redis.getContainerIpAddress(),
        "port=" + redis.getMappedPort());

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.FIELD)
@MapToProperty("spring.redis")
@interface MapToRedisProperties {
}

Callable, Supplier, Optional can be used with any of them. e.g.

@MapToProperty
Supplier<TestPropertyValues> values=()->TestPropertyValues.of("spring-framework.version=5.0.0");

Callable,Supplier and Optional will affect context caching.

Please let me know what you think.

Comment From: antkorwin

@nosan
I think this looks like something which I proposed earlier in my PR #17030 , at least the implementation based on a similar way.

@wilkinsona if you select this approach I will be glad to return working on this implementation. Thanks!

Comment From: nosan

@antkorwin - My approach is based on static final fields with various types. - ContextFactoryCustomizer cannot process properties because we have to be sure that the container's port is available before the context refreshes.

Consider the example:

//if we change the order of the annotations then this 
//test will fail as ContextFactoryCustomizer will be invoked before Container
@Testcontainers
@DataNeo4jTest
class DataNeo4jTestIntegrationTests {

    @Container
    static final Neo4jContainer<?> neo4j = new Neo4jContainer<>();

    @DynamicTestProperty
    private static TestPropertyValues setContainerProperties() {
        return TestPropertyValues.of("spring.data.neo4j.uri=" + neo4j.getBoltUrl);
    }

    @Test
    void name() {

    }
}

That's why I've done everything in ContextCustomizer instead of ContextCustomizerFactory. The limitation of this approach is too difficult to implement the equals and hashCode for ContextCustomizer.

There are two tests to check the lifecycle: - JUnit4 TestRule - JUnit5 Extension

Comment From: philwebb

We discussed this again today briefly and came to the conclusion that we need more time to consider all the implications. Unfortunately, we're all pretty stacked up with Spring Boot 2.2 and Spring Framework 5.2 issues so it's going to have to wait until those releases are done.

We also want to discuss with the Framework team to see if such a feature would make more sense in spring-test. Since there's already a @TestPropertySource annotation, it might be nice if it could be used on a static method (similar to @bsideup's suggestion):

@Testcontainers
@DataRedisTest
public static MyTest {

    @Container
    public static RedisContainer redis = new RedisContainer();

    @TestPropertySource
    static PropertySource propertySource() {
        return TestPropertyValues.of(
            "spring.redis.host=" + redis.getContainerIpAddress(),
            "spring.redis.port=" + redis.getMappedPort()
        );
    }

}

We'd of course need to check that the @Container is setup before the propertySource() method is invoked.

We'll try to talk about these ideas at Spring One. There won't be any decisions until then I'm afraid.

Comment From: antkorwin

@nosan, annotation ordering it's a good point,

I fixed a problem with the order of junit5 extensions, but we aren't able to evaluate value of dynamic properties before run context customizer, this can lead to reset a test context cache even if the test cases used the same properties in the dynamic property provider(property contributor) method.

For example, if we use the dynamic property in the parent abstract class:

public abstract class ParentTest {

    @DynamicTestProperty
    private static TestPropertyValues parentProperties(){
        return TestPropertyValues.of("key=123");
    }
}

and extend this class in multiple tests:

@SpringBootTest
class FirstChildTest extends ParentTest {

    @Value("${key}")
    private String key;

    @Test
    void dynamicPropertyFromSuperClass() {
        assertThat(key).isEqualTo("123");
    }
}

@SpringBootTest
class SecondChildTest extends ParentTest {

    @Value("${key}")
    private String key;

    @Test
    void dynamicPropertyFromSuperClass() {
        assertThat(key).isEqualTo("123");
    }
}

to avoid running a new application context, in this case, I check the equals of method references of dynamic property provider in DynamicTestPropertyContextCustomizer, and context caching works fine.

However in some cases, when a property has the same values but defined by different methods, this will bring to creating a new context in the cache, maybe it will be a limitation of this feature.

I updated my variant of implementation here: https://github.com/spring-projects/spring-boot/compare/master...antkorwin:add-dynamic-test-property

By the way, I can't use the TestPropertySource annotation to define a dynamic property because this annotation has a target level TYPE. This requires making changes in the Spring Framework.

I hope it will be useful..

Comment From: wilkinsona

Following a chat with @sbrannen, I've opened https://github.com/spring-projects/spring-framework/issues/24540 to track this on the Framework side.

Comment From: philwebb

The framework issue has now been fixed and you can define dynamic properties as follows:

@DynamicPropertySource
static void containerProperties(DynamicPropertyRegistry registry) {
    registry.add("test.container.ip", container::getIpAddress);
    registry.add("test.container.port", container::getPort);
}

We'll use this issue to convert our own tests and add some documentation.

Comment From: wilkinsona

I'm not sure how much of a documentation update we need in Boot. The new functionality is now described in Framework's documentation. Usage in Spring Boot would be identical other than replacing @SpringJUnitConfig(/* ... */) with, most likely, @SpringBootTest.

I think we should update our tests to use the new functionality. We can do that in 2.2.x.

Comment From: wilkinsona

Closing in favour of #20676.