spring-cloud / spring-cloud-netflix

Integration with Netflix OSS components
http://cloud.spring.io/spring-cloud-netflix/
Apache License 2.0
4.87k stars 2.44k forks source link

Fixed non-idempotent test `ConfigRefreshTests.verifyGetApplications` #4279

Closed kaiyaok2 closed 3 months ago

kaiyaok2 commented 5 months ago

Motivation:

The test org.springframework.cloud.netflix.eureka.config.ConfigRefreshTests.verifyGetApplications is not idempotent and fails in the second run in the same environment, because it pollutes a state reused by itself. Specifically, it has a mocked EurekaClient instance shared across different test runs. The test verifyGetApplications() checks whether getApplications() is called the correct number of times (3) when a refresh event is fired, and the check fails in the second run as re-execution of publisher.publishEvent() makes more calls to getApplications() on the shared EurekaClient instance. A fix is recommended since unit tests should be idempotent (deterministically passing in repeated runs).

Stacktrace of failure in the second run:

org.mockito.exceptions.verification.TooManyActualInvocations: 
cloudEurekaClient.getApplications();
Wanted 3 times:
-> at com.netflix.discovery.DiscoveryClient.getApplications(DiscoveryClient.java:566)
But was 5 times:
-> at org.springframework.cloud.netflix.eureka.serviceregistry.EurekaServiceRegistry.maybeInitializeClient(EurekaServiceRegistry.java:83)
-> at org.springframework.cloud.netflix.eureka.EurekaDiscoveryClientConfiguration$EurekaClientConfigurationRefresher.onApplicationEvent(EurekaDiscoveryClientConfiguration.java:91)
-> at org.springframework.cloud.netflix.eureka.serviceregistry.EurekaServiceRegistry.maybeInitializeClient(EurekaServiceRegistry.java:83)
-> at org.springframework.cloud.netflix.eureka.EurekaDiscoveryClientConfiguration$EurekaClientConfigurationRefresher.onApplicationEvent(EurekaDiscoveryClientConfiguration.java:91)
-> at org.springframework.cloud.netflix.eureka.serviceregistry.EurekaServiceRegistry.maybeInitializeClient(EurekaServiceRegistry.java:83)
    at com.netflix.discovery.DiscoveryClient.getApplications(DiscoveryClient.java:566)
    at org.springframework.cloud.netflix.eureka.config.ConfigRefreshTests.verifyGetApplications(ConfigRefreshTests.java:55)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Proposed Fix

Add a flag to only invoke publisher.publishEvent() in the first run.

kaiyaok2 commented 3 months ago

@OlgaMaciaszek Changes made. Thanks!