You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2021/11/09 15:36:27 UTC
[GitHub] [camel-quarkus] zhfeng opened a new pull request #3279: Fix #3276 to make sure all objects have been deleted after each test …
zhfeng opened a new pull request #3279:
URL: https://github.com/apache/camel-quarkus/pull/3279
…in AWS2 S3
<!-- Uncomment and fill this section if your PR is not trivial
[ ] An issue should be filed for the change unless this is a trivial change (fixing a typo or similar). One issue should ideally be fixed by not more than one commit and the other way round, each commit should fix just one issue, without pulling in other changes.
[ ] Each commit in the pull request should have a meaningful and properly spelled subject line and body. Copying the title of the associated issue is typically enough. Please include the issue number in the commit message prefixed by #.
[ ] The pull request description should explain what the pull request does, how, and why. If the info is available in the associated issue or some other external document, a link is enough.
[ ] Phrases like Fix #<issueNumber> or Fixes #<issueNumber> will auto-close the named issue upon merging the pull request. Using them is typically a good idea.
[ ] Please run mvn process-resources -Pformat (and amend the changes if necessary) before sending the pull request.
[ ] Contributor guide is your good friend: https://camel.apache.org/camel-quarkus/latest/contributor-guide.html
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] zhfeng merged pull request #3279: Fix #3276 to make sure all objects have been deleted after each test …
Posted by GitBox <gi...@apache.org>.
zhfeng merged pull request #3279:
URL: https://github.com/apache/camel-quarkus/pull/3279
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] zhfeng commented on a change in pull request #3279: Fix #3276 to make sure all objects have been deleted after each test …
Posted by GitBox <gi...@apache.org>.
zhfeng commented on a change in pull request #3279:
URL: https://github.com/apache/camel-quarkus/pull/3279#discussion_r746198352
##########
File path: integration-test-groups/aws2/aws2-s3/src/test/java/org/apache/camel/quarkus/component/aws2/Aws2S3Test.java
##########
@@ -36,6 +37,19 @@
@QuarkusTestResource(Aws2TestResource.class)
class Aws2S3Test {
+ @AfterEach
+ public void after() {
+
+ // Make sure all create objects have been deleted
+ final String[] objects = RestAssured.given()
+ .get("/aws2/s3/object-keys")
+ .then()
+ .statusCode(200)
+ .extract()
+ .body().as(String[].class);
+ Assertions.assertTrue(objects.length == 0);
Review comment:
Good point ! The other issue could be if one of the test leaks the object, and remain tests which running after it will be all failed even if there are no leak objects with them. This might make a confusion to the author. So what about
```java
private int objects_num_before;
private int objects_num_after;
@BeforeEach
public void before() {
objects_num_before = getObjects().length;
}
@AfterEach
public void after(TestInfo testInfo) {
// Make sure all create objects have been deleted after test
objects_num_after = getObjects().length;
if (objects_num_after != objects_num_before) {
Assertions.fail("There is object leak after " + testInfo.getTestMethod().get().getName());
}
}
private String[] getObjects() {
final String[] objects = RestAssured.given()
.get("/aws2/s3/object-keys")
.then()
.statusCode(200)
.extract()
.body().as(String[].class);
return objects;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] aldettinger commented on a change in pull request #3279: Fix #3276 to make sure all objects have been deleted after each test …
Posted by GitBox <gi...@apache.org>.
aldettinger commented on a change in pull request #3279:
URL: https://github.com/apache/camel-quarkus/pull/3279#discussion_r745805878
##########
File path: integration-test-groups/aws2/aws2-s3/src/test/java/org/apache/camel/quarkus/component/aws2/Aws2S3Test.java
##########
@@ -36,6 +37,19 @@
@QuarkusTestResource(Aws2TestResource.class)
class Aws2S3Test {
+ @AfterEach
+ public void after() {
+
+ // Make sure all create objects have been deleted
+ final String[] objects = RestAssured.given()
+ .get("/aws2/s3/object-keys")
+ .then()
+ .statusCode(200)
+ .extract()
+ .body().as(String[].class);
+ Assertions.assertTrue(objects.length == 0);
Review comment:
Is it good idea to put assertions in @AfterEach ?
Would each test be successful only when objects.length ==0, maybe we could assert at the end of each test ?
Would objects.length==0 show a situation where the tests execution is not satisfying, maybe throwing an exception is better ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #3279: Fix #3276 to make sure all objects have been deleted after each test …
Posted by GitBox <gi...@apache.org>.
ppalaga commented on a change in pull request #3279:
URL: https://github.com/apache/camel-quarkus/pull/3279#discussion_r746027414
##########
File path: integration-test-groups/aws2/aws2-s3/src/test/java/org/apache/camel/quarkus/component/aws2/Aws2S3Test.java
##########
@@ -36,6 +37,19 @@
@QuarkusTestResource(Aws2TestResource.class)
class Aws2S3Test {
+ @AfterEach
+ public void after() {
+
+ // Make sure all create objects have been deleted
+ final String[] objects = RestAssured.given()
+ .get("/aws2/s3/object-keys")
+ .then()
+ .statusCode(200)
+ .extract()
+ .body().as(String[].class);
+ Assertions.assertTrue(objects.length == 0);
Review comment:
> Is it good idea to put assertions in @AfterEach ?
My understanding is that this `@AfterEach` assertion is intended much more as a check that the test has no leaks than an ordinary assertion checking the behavior of the application under test. Hence +1 for me.
> Would each test be successful only when objects.length ==0, maybe we could assert at the end of each test ?
`@AfterEach` has the advantage that it holds for all current and future test methods. If a method gets added in the future that leaks objects, the author will get notified early.
> Would objects.length==0 show a situation where the tests execution is not satisfying, maybe throwing an exception is better ?
Well, the assert method throws an AssertionError. Not sure what else could we do for making the test execution fail?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] aldettinger commented on a change in pull request #3279: Fix #3276 to make sure all objects have been deleted after each test …
Posted by GitBox <gi...@apache.org>.
aldettinger commented on a change in pull request #3279:
URL: https://github.com/apache/camel-quarkus/pull/3279#discussion_r746346254
##########
File path: integration-test-groups/aws2/aws2-s3/src/test/java/org/apache/camel/quarkus/component/aws2/Aws2S3Test.java
##########
@@ -36,6 +37,19 @@
@QuarkusTestResource(Aws2TestResource.class)
class Aws2S3Test {
+ @AfterEach
+ public void after() {
+
+ // Make sure all create objects have been deleted
+ final String[] objects = RestAssured.given()
+ .get("/aws2/s3/object-keys")
+ .then()
+ .statusCode(200)
+ .extract()
+ .body().as(String[].class);
+ Assertions.assertTrue(objects.length == 0);
Review comment:
Ok, I see. The intent is like a general assertion that would apply to the set of tests as a whole (not each test individually). So, yes maybe objects_num_after == objects_num_before could be a good fit. Or maybe assume objects_num_before =0. Or maybe record each object ids in a list and check they are not present. All that kind of solutions should be ok I guess.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org