eclipse-vertx / vertx-junit5

Testing Vert.x applications with JUnit 5
Apache License 2.0
42 stars 30 forks source link

Fix numberOfPasses int overflow #48

Closed julianladisch closed 5 years ago

julianladisch commented 5 years ago

numberOfPasses is an int that may overflow. As a result satisfactionTrigger may get invoked several times and overuseTrigger may not get invoked at all. My solution is to stop incrementing once the requiredNumberOfPasses is satisfied.

vietj commented 5 years ago

@jponge can you review and see if we need to back-port to 3.5.4 ?

jponge commented 5 years ago

Sure, but what effect does it have? :-)

Le mar. 25 sept. 2018 à 18:27, julianladisch notifications@github.com a écrit :

@julianladisch commented on this pull request.

In src/test/java/io/vertx/junit5/CountingCheckpointTest.java https://github.com/vert-x3/vertx-junit5/pull/48#discussion_r220260766:

@@ -101,4 +103,43 @@ void check_strict_checkpoint() { .isInstanceOf(IllegalStateException.class) .hasMessage("Strict checkpoint flagged too many times"); } +

  • @Test
  • @EnabledIfEnvironmentVariable(named = "SLOW_UNIT_TESTS", matches = ".*")

This unit tests takes a few minutes depending on the speed of your hardware.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vert-x3/vertx-junit5/pull/48#discussion_r220260766, or mute the thread https://github.com/notifications/unsubscribe-auth/AABlaYNw5ZbTRqszpD0UTrcNG9e2FIfeks5uelmGgaJpZM4W1eKR .

jponge commented 5 years ago

Hi @julianladisch I'd like to clarify the conditional test and see if we need this before moving forward with eventually merging your PR. Thanks!

julianladisch commented 5 years ago

If you don't like the unit tests that were used for test-driven development you can either cherry-pick the main commit 3cee25f, or revert the unit test commit 14b59fe after merging the full pull request.

jponge commented 5 years ago

Thank you @julianladisch, this has been merged in master and 3.6 branches.