-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
op-batcher: Fix intermittency in TestChannelBuilder_PendingFrames_TotalFrames #5989
Conversation
…alFrames Different random seeds would result in varying compression levels which caused intermittency. Now uses a fixed seed for the rng and InsecureRandomKey is now deterministic.
✅ Deploy Preview for opstack-docs canceled.
|
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #5989 +/- ##
==========================================
Coverage ? 40.68%
==========================================
Files ? 319
Lines ? 26142
Branches ? 0
==========================================
Hits ? 10637
Misses ? 14524
Partials ? 981
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This PR has been added to the merge queue, and will be merged soon. |
This PR is next in line to be merged, and will be merged as soon as checks pass. |
This PR is next in line to be merged, and will be merged as soon as checks pass. |
This PR has been added to the merge queue, and will be merged soon. |
This PR is next in line to be merged, and will be merged as soon as checks pass. |
1 similar comment
This PR is next in line to be merged, and will be merged as soon as checks pass. |
Description
Fix intermittency in TestChannelBuilder_PendingFrames_TotalFrames.
Different random seeds would result in varying compression levels in the generated blocks. When the expected compression ratio wasn't reached, the channel may use more frames than the target resulting in a test failure.
Test now uses a fixed seed for the rng and InsecureRandomKey is now deterministic.
InsecureRandomKey
used to useecdsa.GenerateKey
but the Go authors in their infinite wisdom 🙄 made it explicitly non-deterministic even when the rng input is constant - even within the same process. So now it randomly (using the supplied rng) selects a key from a set of 300 hard coded keys.Having unit tests be repeatable is a key requirement for stable builds - if we want to fuzz test, we should explicitly use fuzz testing utils which ensure the entropy seed is clearly logged on a failure so the failing case can be reproduced and debugged.