Skip to content

[DISCUSS] Rewrite S3StorageConnectorTest using testcontainers and MinIO#17539

Merged
rohangarg merged 1 commit intoapache:masterfrom
rohangarg:s3_connector_test_rewrite
Dec 9, 2024
Merged

[DISCUSS] Rewrite S3StorageConnectorTest using testcontainers and MinIO#17539
rohangarg merged 1 commit intoapache:masterfrom
rohangarg:s3_connector_test_rewrite

Conversation

@rohangarg
Copy link
Member

An attempt to rewrite the S3StorageConnectorTest as integration tests using java-testcontainers and MinIO container to simulate S3. This is to facilitate a discussion that whether we should write some of our test using java-testcontainers in future.

Some features of this change :

  1. The tests run on a MinIO server rather than a mock system, thereby testing things more practically
  2. These tests do not run by default locally (via skipping it in local maven profile), but always run on the CI (via CI profile)

)
)
.withCredentials(new StaticCredentialsProvider(
new BasicAWSCredentials(container.getUserName(), "wrong")))

Check failure

Code scanning / CodeQL

Hard-coded credential in API call

Hard-coded value flows to [sensitive API call](1).
@gianm
Copy link
Contributor

gianm commented Dec 6, 2024

I just tried this locally and it worked like a charm while running Docker Desktop. The test suite ran in less than a second. IMO this is a lot nicer than using mocks or ITs.

Copy link
Member

@clintropolis clintropolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@rohangarg rohangarg merged commit ae4ea51 into apache:master Dec 9, 2024
@adarshsanjeev adarshsanjeev added this to the 32.0.0 milestone Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants