-
Notifications
You must be signed in to change notification settings - Fork 18
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
Release/v0.1.22 #84
Release/v0.1.22 #84
Conversation
* Add proper timeout error handling * Fix lint errors --------- Co-authored-by: Samuel Benattar <[email protected]>
Co-authored-by: Sergei Makarov <[email protected]>
@samybenatt hello. I hope you are doing well. Could you please review this PR? |
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.
Hey @siller174
I reviewed all your implementation
I think one key difference between your implementation and mine is that, when one of the attempt is successful, I mark the whole test as successful, and I don't keep error history. I am only failing if the last attempt is still failing.
In your implementation, the test will start failing even after retrying.
The optional and broken error options are not really fulfilling the intended purpose because it is either setting all failures as broken/optional or all failures as failed. But the retry purpose is the following:
- The test will be retried up to
MaxAttempts
times - The retries will only be executed if the test is having errors
- If the test is successful at any iteration between attempt 1 and
MaxAttempts
, the loop will break and return the overall test result as successful (it will not process previous errors and it will never trigger t.Fail()) - The status of the overall test (success or fail) will be based on either the first attempt that is successful, or, if no attempt is successful, it will be based on the latest execution result
Retry
testRequestRepeat*
toRequestRetry*