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.
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
Added suspendForMs to @RetryingTest #604
Added suspendForMs to @RetryingTest #604
Conversation
@@ -167,6 +192,9 @@ public RetryingTestInvocationContext next() { | |||
if (!hasNext()) | |||
throw new NoSuchElementException(); | |||
retriesSoFar++; | |||
|
|||
suspendFor(suspendForMs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should not wait, if this is the first execution of the test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mathieufortin01, thank you for the PR, I really appreciate it! If you've observed the project a bit, you know we occasionally take some time, but we're on it now.
Can you merge main
, resolve the conflict and take a look at these comments?
src/main/java/org/junitpioneer/jupiter/RetryingTestExtension.java
Outdated
Show resolved
Hide resolved
src/test/java/org/junitpioneer/jupiter/RetryingTestExtensionTests.java
Outdated
Show resolved
Hide resolved
src/test/java/org/junitpioneer/jupiter/RetryingTestExtensionTests.java
Outdated
Show resolved
Hide resolved
@nipafx Thanks for the review. Addressed all comments. Let me know. |
src/main/java/org/junitpioneer/jupiter/RetryingTestExtension.java
Outdated
Show resolved
Hide resolved
if (!isFirstExecution()) { | ||
suspendFor(suspendForMs); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this little refactoring! 👍🏾
Looks great, thank you @mathieufortin01! I was about to merge when I noticed that you didn't add yourself to the README - we like to list all contributors there to publicly |
@nipafx here you go. let the shame begin. |
Commit message:
|
Thanks a lot @mathieufortin01, for your contribution and patience! 👏🏾👏🏾👏🏾 |
…it-pioneer#604) Retrying tests may be flaky because of issues with external services. The chance that they recover and behave correctly on the next try may improve if there's a (short) pause between the test runs. This change adds an option to suspend for a fixed time span (in milliseconds) betweet tries. The implemenation simply lets the executing thread sleep, which isn't ideal. It would be much better if there were a way to schedule the next execution for later and use the thread to execute other tests in between, but Jupiter doesn't offer an API for that. Closes: junit-pioneer#407 PR: junit-pioneer#604
fixes #407
Simple implementation just to get the discussion going.
suspendForMs
param to@RetryingTest
Proposed commit message:
PR checklist
The following checklist shall help the PR's author, the reviewers and maintainers to ensure the quality of this project.
It is based on our contributors guidelines, especially the "writing code" section.
It shall help to check for completion of the listed points.
If a point does not apply to the given PR's changes, the corresponding entry can be simply marked as done.
Documentation (general)
.adoc
file in thedocs
folder, e.g.docs/report-entries.adoc
.adoc
file references demo insrc/demo/java
instead of containing code blocks as text.adoc
files)Documentation (new extension)
docs/docs-nav.yml
navigation has an entry for the new extensionpackage-info.java
contains information about the new extensionCode
Contributing
README.md
mentions the new contribution (real name optional)I hereby agree to the terms of the JUnit Pioneer Contributor License Agreement.