Update example to include a withoutSlack option #96
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In #90 @twelsh-aw found a bug in a new implementation. This turned out to be caused by us mocking time. Since time mocking will always have some risks this diff proposes to expand the `examples` we're using - since they use "real" time they should be good enough to cover most basic cases like #90. In particular: - we add a "withoutSlack" test so that the exact case reported in #90 doesn't happen. No slack option seems common enough to add it. - updates "withSlack" example to actually show how slack operates. Due to non-even execution times I'm forced to round the time a bit. Possible issues: - test stability: I re-run the test a 1000 times without issues - the timing seems to be stable. - test duration: in total we're extending the examples by 5ms, which shouldn't be human noticeable.
Codecov Report
@@ Coverage Diff @@
## main #96 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 97 97
=========================================
Hits 97 97 Continue to review full report at Codecov.
|
|
Hey @storozhukBM WDYT? |
|
@rabbbit looks good |
storozhukBM
approved these changes
Jul 6, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #90 @twelsh-aw found a bug in a new implementation. This turned out
to be caused by us mocking time.
Since time mocking will always have some risks this diff proposes
to expand the
exampleswe're using - since they use "real" timethey should be good enough to cover most basic cases like #90.
In particular:
doesn't happen. No slack option seems common enough to add it.
Due to non-even execution times I'm forced to round the time a bit.
Possible issues:
timing seems to be stable.
shouldn't be human noticeable.