Disable @ConfigurationProperties scan in slice tests

[ebussieres]

I've found a regression with version 2.2.0.M2.

I have a dao test which is using @DataJpaTest annotation that looks like this :

@DataJpaTest
@RunWith(SpringRunner.class)
@Sql(executionPhase = BEFORE_TEST_METHOD, scripts = "classpath:beforeTestDaoITest.sql")
@Sql(executionPhase = AFTER_TEST_METHOD, scripts = "classpath:afterTestDaoITest.sql")
public class TestDaoITest {
  @Autowired
  private TestDao underTest;

  @Test
  public void whenFindOne_givenExistingId_thenId() {
    Optional<TestEntity> result = this.underTest.findById(1L);

    assertEquals(result.get().id, (Long) 1L);
  }
}

In the project, I have a configuration class which use @ConfigurationProperties and it looks like this:

@Configuration
@ConfigurationProperties(prefix = "monitoring")
public class Config {
  private Long id;

  @Bean
  public HealthIndicator healthIndicator(HealthAggregator healthAggregator) {
    HealthIndicatorRegistry healthIndicatorRegistry = new DefaultHealthIndicatorRegistry();
    return new CompositeHealthIndicator(healthAggregator, healthIndicatorRegistry);
  }

  public Long getId() {
    return this.id;
  }

  public void setId(Long id) {
    this.id = id;
  }
}

With version 2.1.4.RELEASE, everything works fine. Because it's a DataJpaTest, the Config isn't scanned and the test passes.

With version 2.2.0.M2, the config class is scanned and an error occured in the test because no HealthAggregator bean exists. I think that the reason comes from @ConfigurationProperties, When I remove the @ConfigurationProperties annotation, the config class isn't scanned anymore

I have a working example here that reproduce the problem : https://github.com/ebussieres/spring-boot-2.2-issue

[wilkinsona]

Thanks for the sample and for trying out a 2.2 milestone. The problem's an interesting side-effect of the new configuration property scanning support.

It's recommended that classes annotated with @ConfigurationProperties are POJOs and don't provide any other functionality. They shouldn't also be a @Configuration class or a @RestController for example. Your code doesn't follow this recommendation which is why the failure's occurring.

I don't think it'll be possible to make configuration property scanning aware of test slices as there's no obvious way of knowing which @ConfigurationProperties classes are needed in a particular slice of the application. Let's see if there's anything else we can do.

[ebussieres]

I'll check to change our configuration classes for now, I was not aware of that recommendation. Thanks for your quick answer.

[wilkinsona]

@snicoll pointed out that in 2.1 without @ConfigurationPropertiesScan no @ConfigurationProperties beans will be created until they are enabled via @EnableConfigurationProperties on the main class. If we don't do any configuration property scanning when running a slice test, we will restore the same behaviour as we have in 2.1.

[wilkinsona]

@mbhave has pointed out that we overlooked the scenario where @EnableConfigurationProperties was on the main class in 2.1 and was then removed in favour of scanning in 2.2. In this case, the current behaviour of configuration property scanning matches 2.1 and disabling it would create a problem.

[snicoll]

In this case, the current behaviour of configuration property scanning matches 2.1 and disabling it would create a problem.

I might be missing something but I don't see this as a problem. For that to happen, the user has to change something in their code (rely on classpath scanning vs. regular registration). If they do that, it is fair game IMO that the @ConfigurationProperties bean goes away. If they don't change anything, tests will work as before.

I know the analogy is not 100% correct but if you move a @Bean method on the main class to @Component on the target object, it will go away too. If you need to get those @ConfigurationProperties bean, you need a way to indicate them and, at the moment, I don't think there is a good use case for scanning them in a slice tests.

[wilkinsona]

If they do that, it is fair game IMO that the @ConfigurationProperties bean goes away

My concern is that it undermines the benefit of moving to configuration property scanning by, potentially, making test slices harder to use. Getting rid of one @EnableConfiguraionProperties annotation in your main code only to have to add it to your test code seems like a bit of a mixed bag. On the plus side, it does provide a bit more control.

[snicoll]

making test slices harder to use.

How is that any different from having to mock a bean that the component you want to test needs exactly? Slice tests run with a reduced classpath so I really don't see why we should consider @ConfigurationProperties type.

[wilkinsona]

It's different purely because it's something that you didn't have to do before. For that reason, I think it detracts a little bit from the benefits of moving to configuration property scanning.