test: enable integration tests against real S3 (fixes #1899) #1951
Conversation
iwasakims
requested review from
bscholtes1A,
jimmarino and
paullatzelsperger
as code owners
| private static boolean isMinio() { | ||
| return MINIO_ENDPOINT.equals(S3_ENDPOINT.toString()); | ||
| } | ||
|
|
||
| /** | ||
| * pings <a href="https://docs.min.io/minio/baremetal/monitoring/healthcheck-probe.html">MinIO's health endpoint</a> | ||
| * | ||
| * @return true if HTTP status [200..300[ | ||
| */ | ||
| private static boolean pingMinio() throws IOException { |
There was a problem hiding this comment.
At this point I would rename this method, the name is not consistent and it does not explain the result.
Could be refactored in something like:
@BeforeAll
static void prepareAll() {
await().atLeast(Duration.ofSeconds(2))
.atMost(Duration.ofSeconds(15))
.with()
.pollInterval(Duration.ofSeconds(2))
.ignoreException(IOException.class) // thrown by minioIsAvailable
.ignoreException(ConnectException.class)
.until(() -> {
if (isMinio()) {
return minioIsAvailable();
} else {
return true;
}
});
}
private static boolean isMinio() {
return MINIO_ENDPOINT.equals(S3_ENDPOINT.toString());
}
/**
* pings <a href="https://docs.min.io/minio/baremetal/monitoring/healthcheck-probe.html">MinIO's health endpoint</a>
*
* @return true if HTTP status [200..300[
*/
private static boolean minioIsAvailable() throws IOException {
var httpClient = new OkHttpClient();
var healthRq = new Request.Builder().url(S3_ENDPOINT + "/minio/health/live").get().build();
try (var response = httpClient.newCall(healthRq).execute()) {
return response.isSuccessful();
}
}There was a problem hiding this comment.
I updated the code based on your suggestion. I chose isMinioAvailable preferring is* for the name of method returning boolean. @ndr-brt
| @@ -158,7 +166,7 @@ protected void putStringOnBucket(String bucketName, String key, String content) | |||
| } | |||
|
|
|||
| protected @NotNull AwsCredentials getCredentials() { | |||
| String profile = propOrEnv("AWS_PROFILE", null); | |||
| String profile = propOrEnv("it.aws.profile", null); | |||
|
before I review this, please fix the failing tests. |
|
@paullatzelsperger The cause seems to be running S3DataPlaneIntegrationTest without starting Minio in unexpected jobs like Postgresql-Integration-Tests and Hashicorp-Vault-Integration-Tests. I expect #1897 to address this. Let me rebase this PR and address review comment of @ndr-brt. |
|
The cause of the test failure is change of build.gradle.kts overwriting test task setting for skipping IntegrationTests. I will revert the change of build.gradle.kts. We can use environment variable instead of system properties without fixing build.gradle.kts. |
| @@ -83,15 +85,25 @@ static void prepareAll() { | |||
| .pollInterval(Duration.ofSeconds(2)) | |||
| .ignoreException(IOException.class) // thrown by pingMinio | |||
| .ignoreException(ConnectException.class) | |||
| .until(AbstractS3Test::pingMinio); | |||
| .until(() -> { | |||
There was a problem hiding this comment.
minor nit: introducing a method, something like isBackendAvailable or similar naming would make this piece of code more readable:
.ignoreException(ConnectException.class)
.until(this::isBackendAvailable);
}
// and then:
private boolean isBackendAvailable(){
if(isMinio()){
return isMinioAvailable();
} else {
return true;
}
};There was a problem hiding this comment.
Sure. I updated the code.
Codecov Report
@@ Coverage Diff @@
## main #1951 +/- ##
==========================================
+ Coverage 62.78% 62.88% +0.10%
==========================================
Files 781 782 +1
Lines 16608 16640 +32
Branches 1080 1081 +1
==========================================
+ Hits 10427 10464 +37
+ Misses 5730 5726 -4
+ Partials 451 450 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
What this PR changes/adds
Make S3 endpoint configurable via system properties.
Why it does that
For running integration tests against real S3 instead of Minio.
Further notes
I ran tests against Tokyo region with the following command lines using my AWS credential.
and.
Linked Issue(s)
Closes #1899
Checklist
no-changelog)