Skip to content

Commit

Permalink
fix(artifacts/test): clear the ArtifactStore.instance in S3ArtifactSt… (
Browse files Browse the repository at this point in the history
#1137)

* fix(artifacts/test): clear the ArtifactStore.instance in S3ArtifactStoreConfigurationTest

so other tests that assume ArtifactStore.getInstance is null (e.g. ExpectedArtifactTest) don't try to store an artifact and fail trying to communicate with s3 with an error like:

software.amazon.awssdk.core.exception.SdkClientException: Unable to marshall request to JSON: Parameter 'Bucket' must not be null
	at software.amazon.awssdk.core.exception.SdkClientException$BuilderImpl.build(SdkClientException.java:111)
	at software.amazon.awssdk.services.s3.transform.HeadObjectRequestMarshaller.marshall(HeadObjectRequestMarshaller.java:53)
	at software.amazon.awssdk.services.s3.transform.HeadObjectRequestMarshaller.marshall(HeadObjectRequestMarshaller.java:31)
	at software.amazon.awssdk.core.internal.handler.BaseClientHandler.lambda$finalizeSdkHttpFullRequest$0(BaseClientHandler.java:73)
	at software.amazon.awssdk.core.internal.util.MetricUtils.measureDuration(MetricUtils.java:50)
	at software.amazon.awssdk.core.internal.handler.BaseClientHandler.finalizeSdkHttpFullRequest(BaseClientHandler.java:72)
	at software.amazon.awssdk.core.internal.handler.BaseSyncClientHandler.doExecute(BaseSyncClientHandler.java:151)
	at software.amazon.awssdk.core.internal.handler.BaseSyncClientHandler.lambda$execute$1(BaseSyncClientHandler.java:82)
	at software.amazon.awssdk.core.internal.handler.BaseSyncClientHandler.measureApiCallSuccess(BaseSyncClientHandler.java:179)
	at software.amazon.awssdk.core.internal.handler.BaseSyncClientHandler.execute(BaseSyncClientHandler.java:76)
	at software.amazon.awssdk.core.client.handler.SdkSyncClientHandler.execute(SdkSyncClientHandler.java:45)
	at software.amazon.awssdk.awscore.client.handler.AwsSyncClientHandler.execute(AwsSyncClientHandler.java:56)
	at software.amazon.awssdk.services.s3.DefaultS3Client.headObject(DefaultS3Client.java:5438)
	at com.netflix.spinnaker.kork.artifacts.artifactstore.s3.S3ArtifactStoreStorer.objectExists(S3ArtifactStoreStorer.java:132)
	at com.netflix.spinnaker.kork.artifacts.artifactstore.s3.S3ArtifactStoreStorer.store(S3ArtifactStoreStorer.java:90)
	at com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStore.store(ArtifactStore.java:39)
	at com.netflix.spinnaker.kork.artifacts.model.ExpectedArtifact.store(ExpectedArtifact.java:160)
	at com.netflix.spinnaker.kork.artifacts.model.ExpectedArtifact.<init>(ExpectedArtifact.java:56)
	at com.netflix.spinnaker.kork.artifacts.model.ExpectedArtifact$ExpectedArtifactBuilder.build(ExpectedArtifact.java:44)
	at com.netflix.spinnaker.kork.artifacts.model.ExpectedArtifactTest.testReferenceMatches(ExpectedArtifactTest.java:205)

Yes, it's possible to configure a bucket name in S3ArtifactStoreConfiguration, but the more fundamental issue is having tests leaving a clean slate when they're done.

* refactor(artifacts/test): use @AfterEach to protect against other tests polluting state
  • Loading branch information
dbyron-sf authored Dec 18, 2023
1 parent ad67631 commit ec69211
Showing 1 changed file with 12 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@
import com.netflix.spinnaker.kork.artifacts.artifactstore.ArtifactStoreURIBuilder;
import com.netflix.spinnaker.kork.artifacts.artifactstore.NoopArtifactStoreGetter;
import com.netflix.spinnaker.kork.artifacts.artifactstore.NoopArtifactStoreStorer;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInfo;
import org.springframework.boot.context.annotation.UserConfigurations;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
import org.springframework.test.util.ReflectionTestUtils;
import software.amazon.awssdk.services.s3.S3Client;

class S3ArtifactStoreConfigurationTest {
Expand All @@ -41,6 +43,16 @@ void init(TestInfo testInfo) {
System.out.println("--------------- Test " + testInfo.getDisplayName());
}

@AfterEach
void cleanup() {
// Clear ArtifactStore.instance so we don't leave lingering state for other
// tests that assume it's null.
//
// Note that ArtifactStore.setInstance(null) doesn't set instance to null
// if it's already set.
ReflectionTestUtils.setField(ArtifactStore.class, "instance", null);
}

@Test
void testArtifactStoreS3Disabled() {
runner
Expand Down

0 comments on commit ec69211

Please sign in to comment.