Skip to content

Commit 6317464

Browse files
committed
ci: fix cleanup-after-class code
- CloudFoundryCleaner was not properly triggered on every test. In fact, it was only triggered when the integration test's TestContext would change, which was very infrequent (unsure it was ever triggered). It's possible the cleanup only ever happened when the test suite started. - This is because the "clean" method was linked to lifecycle methods of the CloudFoundryCleaner bean, which was only created once. - The solution is to add `@DirtiesContext`, to trigger a re-creation of this bean. We don't want to do it on every test, because it could add more that 1h to the whole test suite. To ensure that it is well understood why this annotation is there in the first place, we introduce the meta-annotation @CleanupCloudFoundryAfterClass. - Note that calling CloudFoundryCleaner#clean() directly in tests would not work, because that annotation deletes EVERYTHING, and then subsequent tests would not have an org, a space, etc to operate in. By using @DirtiesContext we ensure those other beans are recreated as well. - Lastly, I have a doubt that the CloudFoundryCleaner is even required. I believe deleting an org in current versions of CF deletes EVERYTHING within this org, so it should be enough. However, this would be a much bigger change to the tests, which we don't have the bandwidth to tackle at the moment.
1 parent cefcd2a commit 6317464

15 files changed

+80
-8
lines changed
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package org.cloudfoundry;
2+
3+
import java.lang.annotation.ElementType;
4+
import java.lang.annotation.Retention;
5+
import java.lang.annotation.RetentionPolicy;
6+
import java.lang.annotation.Target;
7+
import org.springframework.context.annotation.Bean;
8+
import org.springframework.test.annotation.DirtiesContext;
9+
10+
/**
11+
* Meta-annotation to show that a test class will add too much to the CF instance, and that a full universe
12+
* cleanup should occur. This is important because otherwise the integration tests create too many apps and
13+
* blow up the memory quota. We do not want to recreate the full environment for EVERY test class because
14+
* the process takes 30~60s, so in total it could add more than an hour to the integration tests.
15+
* <p>
16+
* Technically, this is achieved by recreating a Spring ApplicationContext with {@link DirtiesContext}. The
17+
* {@link CloudFoundryCleaner} bean will be destroyed, which triggers {@link CloudFoundryCleaner#clean()}.
18+
* After that, every {@link Bean} in {@link IntegrationTestConfiguration} is recreated, including users,
19+
* clients, organizations, etc: everything required to run a test.
20+
* <p>
21+
* We use a meta-annotation instead of a raw {@link DirtiesContext} to make it clear what it does, rather
22+
* than having to understand complicated lifecycle issues.
23+
*/
24+
@Retention(RetentionPolicy.RUNTIME)
25+
@Target(ElementType.TYPE)
26+
@DirtiesContext
27+
public @interface CleanupCloudFoundryAfterClass {}

integration-test/src/test/java/org/cloudfoundry/CloudFoundryCleaner.java

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,16 @@
116116
import org.cloudfoundry.util.ResourceUtils;
117117
import org.slf4j.Logger;
118118
import org.slf4j.LoggerFactory;
119+
import org.springframework.beans.factory.DisposableBean;
120+
import org.springframework.beans.factory.InitializingBean;
119121
import reactor.core.publisher.Flux;
120122
import reactor.core.publisher.Mono;
121123
import reactor.util.function.Tuples;
122124
import reactor.util.retry.Retry;
123125

124-
final class CloudFoundryCleaner {
126+
final class CloudFoundryCleaner implements InitializingBean, DisposableBean {
127+
128+
private static boolean cleanSlateEnvironment = false;
125129

126130
private static final Logger LOGGER = LoggerFactory.getLogger("cloudfoundry-client.test");
127131

@@ -162,6 +166,25 @@ final class CloudFoundryCleaner {
162166
this.uaaClient = uaaClient;
163167
}
164168

169+
/**
170+
* Once at the beginning of the whole test suite, clean up the environment. It should only ever happen
171+
* once, hence the static init variable.
172+
*/
173+
@Override
174+
public void afterPropertiesSet() {
175+
if (!cleanSlateEnvironment) {
176+
LOGGER.info(
177+
"Performing clean slate cleanup. Should happen once per integration test run.");
178+
this.clean();
179+
cleanSlateEnvironment = true;
180+
}
181+
}
182+
183+
@Override
184+
public void destroy() {
185+
this.clean();
186+
}
187+
165188
void clean() {
166189
Flux.empty()
167190
.thenMany(

integration-test/src/test/java/org/cloudfoundry/IntegrationTestConfiguration.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import java.util.Comparator;
3838
import java.util.HashMap;
3939
import java.util.List;
40-
import java.util.Random;
4140
import org.cloudfoundry.client.CloudFoundryClient;
4241
import org.cloudfoundry.client.v2.info.GetInfoRequest;
4342
import org.cloudfoundry.client.v2.organizationquotadefinitions.CreateOrganizationQuotaDefinitionRequest;
@@ -243,14 +242,13 @@ String clientSecret(NameFactory nameFactory) {
243242
return nameFactory.getClientSecret();
244243
}
245244

246-
@Bean(initMethod = "clean", destroyMethod = "clean")
245+
@Bean
247246
CloudFoundryCleaner cloudFoundryCleaner(
248247
@Qualifier("admin") CloudFoundryClient cloudFoundryClient,
249248
NameFactory nameFactory,
250249
@Qualifier("admin") NetworkingClient networkingClient,
251250
Version serverVersion,
252251
@Qualifier("admin") UaaClient uaaClient) {
253-
254252
return new CloudFoundryCleaner(
255253
cloudFoundryClient, nameFactory, networkingClient, serverVersion, uaaClient);
256254
}

integration-test/src/test/java/org/cloudfoundry/client/v2/ApplicationsTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
3737
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
3838
import org.cloudfoundry.AbstractIntegrationTest;
39+
import org.cloudfoundry.CleanupCloudFoundryAfterClass;
3940
import org.cloudfoundry.CloudFoundryVersion;
4041
import org.cloudfoundry.IfCloudFoundryVersion;
4142
import org.cloudfoundry.client.CloudFoundryClient;
@@ -96,6 +97,7 @@
9697
import reactor.test.StepVerifier;
9798
import reactor.util.function.Tuple2;
9899

100+
@CleanupCloudFoundryAfterClass
99101
public final class ApplicationsTest extends AbstractIntegrationTest {
100102

101103
@Autowired private CloudFoundryClient cloudFoundryClient;

integration-test/src/test/java/org/cloudfoundry/client/v2/ServiceBrokersTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.time.Duration;
2626
import org.cloudfoundry.AbstractIntegrationTest;
2727
import org.cloudfoundry.ApplicationUtils;
28+
import org.cloudfoundry.CleanupCloudFoundryAfterClass;
2829
import org.cloudfoundry.ServiceBrokerUtils;
2930
import org.cloudfoundry.client.CloudFoundryClient;
3031
import org.cloudfoundry.client.v2.servicebrokers.CreateServiceBrokerRequest;
@@ -43,6 +44,7 @@
4344
import reactor.core.publisher.Mono;
4445
import reactor.test.StepVerifier;
4546

47+
@CleanupCloudFoundryAfterClass
4648
public final class ServiceBrokersTest extends AbstractIntegrationTest {
4749

4850
@Autowired private CloudFoundryClient cloudFoundryClient;

integration-test/src/test/java/org/cloudfoundry/client/v3/AdminTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.cloudfoundry.util.JobUtils;
2727
import org.junit.jupiter.api.Test;
2828
import org.springframework.beans.factory.annotation.Autowired;
29-
3029
import reactor.core.publisher.Mono;
3130
import reactor.test.StepVerifier;
3231

integration-test/src/test/java/org/cloudfoundry/client/v3/ApplicationsTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.Collections;
2929
import java.util.Map;
3030
import org.cloudfoundry.AbstractIntegrationTest;
31+
import org.cloudfoundry.CleanupCloudFoundryAfterClass;
3132
import org.cloudfoundry.CloudFoundryVersion;
3233
import org.cloudfoundry.IfCloudFoundryVersion;
3334
import org.cloudfoundry.client.CloudFoundryClient;
@@ -106,6 +107,7 @@
106107
import reactor.test.StepVerifier;
107108
import reactor.util.function.Tuples;
108109

110+
@CleanupCloudFoundryAfterClass
109111
public final class ApplicationsTest extends AbstractIntegrationTest {
110112

111113
@Autowired private CloudFoundryClient cloudFoundryClient;

integration-test/src/test/java/org/cloudfoundry/client/v3/DeploymentsTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.nio.file.Path;
2323
import java.time.Duration;
2424
import org.cloudfoundry.AbstractIntegrationTest;
25+
import org.cloudfoundry.CleanupCloudFoundryAfterClass;
2526
import org.cloudfoundry.CloudFoundryVersion;
2627
import org.cloudfoundry.IfCloudFoundryVersion;
2728
import org.cloudfoundry.client.CloudFoundryClient;
@@ -51,6 +52,7 @@
5152
import reactor.test.StepVerifier;
5253

5354
@IfCloudFoundryVersion(greaterThanOrEqualTo = CloudFoundryVersion.PCF_2_4)
55+
@CleanupCloudFoundryAfterClass
5456
public final class DeploymentsTest extends AbstractIntegrationTest {
5557

5658
@Autowired private CloudFoundryClient cloudFoundryClient;

integration-test/src/test/java/org/cloudfoundry/client/v3/ProcessesTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.nio.file.Path;
2323
import java.time.Duration;
2424
import org.cloudfoundry.AbstractIntegrationTest;
25+
import org.cloudfoundry.CleanupCloudFoundryAfterClass;
2526
import org.cloudfoundry.CloudFoundryVersion;
2627
import org.cloudfoundry.IfCloudFoundryVersion;
2728
import org.cloudfoundry.client.CloudFoundryClient;
@@ -42,6 +43,7 @@
4243
import reactor.test.StepVerifier;
4344

4445
@IfCloudFoundryVersion(greaterThanOrEqualTo = CloudFoundryVersion.PCF_2_0)
46+
@CleanupCloudFoundryAfterClass
4547
public final class ProcessesTest extends AbstractIntegrationTest {
4648

4749
@Autowired private CloudFoundryClient cloudFoundryClient;

integration-test/src/test/java/org/cloudfoundry/client/v3/ServiceBrokersTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.time.Duration;
2626
import org.cloudfoundry.AbstractIntegrationTest;
2727
import org.cloudfoundry.ApplicationUtils;
28+
import org.cloudfoundry.CleanupCloudFoundryAfterClass;
2829
import org.cloudfoundry.CloudFoundryVersion;
2930
import org.cloudfoundry.IfCloudFoundryVersion;
3031
import org.cloudfoundry.ServiceBrokerUtils;
@@ -49,6 +50,7 @@
4950
import reactor.test.StepVerifier;
5051

5152
@IfCloudFoundryVersion(greaterThanOrEqualTo = CloudFoundryVersion.PCF_2_10)
53+
@CleanupCloudFoundryAfterClass
5254
public final class ServiceBrokersTest extends AbstractIntegrationTest {
5355

5456
@Autowired private CloudFoundryClient cloudFoundryClient;

0 commit comments

Comments
 (0)