Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix #3728 AWS S3 integration test should remove all objects in finall… #3729

Merged
merged 1 commit into from
Apr 15, 2022
Merged

Fix #3728 AWS S3 integration test should remove all objects in finall… #3729

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
117 changes: 63 additions & 54 deletions .../aws2/aws2-s3/src/test/java/org/apache/camel/quarkus/component/aws2/s3/it/Aws2S3Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,47 +164,51 @@ public void testKms() throws Exception {
final String oid = UUID.randomUUID().toString();
final String blobContent = "Hello KMS " + oid;

// Create
RestAssured.given()
.contentType(ContentType.TEXT)
.body(blobContent)
.post("/aws2/s3/object/" + oid + "?useKms=true")
.then()
.statusCode(201);

// Read
RestAssured.get("/aws2/s3/object/" + oid + "?useKms=true")
.then()
.statusCode(200)
.body(is(blobContent));
try {
// Create
RestAssured.given()
.contentType(ContentType.TEXT)
.body(blobContent)
.post("/aws2/s3/object/" + oid + "?useKms=true")
.then()
.statusCode(201);

// Delete
deleteObject(oid);
// Read
RestAssured.get("/aws2/s3/object/" + oid + "?useKms=true")
.then()
.statusCode(200)
.body(is(blobContent));
} finally {
// Delete
deleteObject(oid);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be done in an AfterEach method instead of a finally block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The oid is a random generated id and there are some other tests such as copyObjectDeleteBucket which has a different test logic. There's also a @AfterEach method to check the objects leaking.

Copy link
Contributor

Choose a reason for hiding this comment

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

@essobedo thanks for the review!
We were hit by high AWS bills in the past when tests were either missing cleanup code or when something failed midair and the cleanup code was not invoked. Using try-catch helps to address both: missing cleanup code is easier to spot and existing cleanup code is harder to skip. Thus +1 for this change from my side.

}
}

@Test
public void upload() throws Exception {
final String oid = UUID.randomUUID().toString();
final String content = RandomStringUtils.randomAlphabetic(8 * 1024 * 1024);

RestAssured.given()
.contentType(ContentType.TEXT)
.body(content)
.post("/aws2/s3/upload/" + oid)
.then()
.statusCode(200);

String result = RestAssured.get("/aws2/s3/object/" + oid)
.then()
.statusCode(200)
.extract().asString();
try {
RestAssured.given()
.contentType(ContentType.TEXT)
.body(content)
.post("/aws2/s3/upload/" + oid)
.then()
.statusCode(200);

// Delete
deleteObject(oid);
String result = RestAssured.get("/aws2/s3/object/" + oid)
.then()
.statusCode(200)
.extract().asString();

// strip the chuck-signature
result = result.replaceAll("\\s*[0-9]+;chunk-signature=\\w{64}\\s*", "");
assertEquals(content, result);
// strip the chuck-signature
result = result.replaceAll("\\s*[0-9]+;chunk-signature=\\w{64}\\s*", "");
assertEquals(content, result);
} finally {
// Delete
deleteObject(oid);
}
}

@Test
Expand Down Expand Up @@ -284,39 +288,44 @@ public void downloadLink() throws Exception {
final String oid = UUID.randomUUID().toString();
final String blobContent = "Hello " + oid;

// Create
createObject(oid, blobContent);
try {
// Create
createObject(oid, blobContent);

// Download link
RestAssured.given()
.contentType(ContentType.TEXT)
.get("/aws2/s3/downloadlink/" + oid)
.then()
.statusCode(200);
// Download link
RestAssured.given()
.contentType(ContentType.TEXT)
.get("/aws2/s3/downloadlink/" + oid)
.then()
.statusCode(200);

// Delete
deleteObject(oid);
} finally {
// Delete
deleteObject(oid);
}
}

@Test
public void objectRange() {
final String oid = UUID.randomUUID().toString();
final String blobContent = "Hello " + oid;

// Create
createObject(oid, blobContent);

// Object range
RestAssured.given()
.contentType(ContentType.TEXT)
.param("start", "0").param("end", "4")
.get("/aws2/s3/object/range/" + oid)
.then()
.statusCode(200)
.body(is("Hello"));
try {
// Create
createObject(oid, blobContent);

// Delete
deleteObject(oid);
// Object range
RestAssured.given()
.contentType(ContentType.TEXT)
.param("start", "0").param("end", "4")
.get("/aws2/s3/object/range/" + oid)
.then()
.statusCode(200)
.body(is("Hello"));
} finally {
// Delete
deleteObject(oid);
}
}

private void createObject(String oid, String blobContent) {
Expand Down