-
Notifications
You must be signed in to change notification settings - Fork 382
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
fix: Improve BFT Tests Stability #1385
Conversation
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1385 +/- ##
==========================================
- Coverage 56.30% 56.11% -0.20%
==========================================
Files 422 413 -9
Lines 65699 64129 -1570
==========================================
- Hits 36993 35983 -1010
+ Misses 25821 25318 -503
+ Partials 2885 2828 -57 ☔ View full report in Codecov by Sentry. |
40a6278
to
827efc0
Compare
Signed-off-by: gfanton <[email protected]>
827efc0
to
a2e6404
Compare
Approving for a green CI! 🎉 |
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.
Please open an issue, mandatory for the mainnet to address the root cause.
We should revert this: #1320 (comment) |
This reverts commit 63ff15b. Signed-off-by: gfanton <[email protected]>
This PR reverts #1385 following @jaekwon #1320 (comment) <!-- please provide a detailed description of the changes made in this pull request. --> <details><summary>Contributors' checklist...</summary> - [ ] Added new tests, or not needed, or not feasible - [ ] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [ ] Updated the official documentation or not needed - [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [ ] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details> Signed-off-by: gfanton <[email protected]>
ref gnolang#1320 This draft PR demonstrates a way for fixing flaky BFT tests. As mentioned in gnolang#1320 the main issue is that some channels are not fully consumed, leading to deadlocks. By using buffered channels as proxy in state tests, we can easily avoid these deadlocks. However, I don't think this is the best approach, as these channels were originally set to zero (capacity) for good reasons, but perhaps it could make sense for testing purposes. In any case, if anyone has a better idea for a solution, your suggestions are welcome. <details><summary>Contributors' checklist...</summary> - [ ] Added new tests, or not needed, or not feasible - [ ] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [ ] Updated the official documentation or not needed - [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [ ] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details> --------- Signed-off-by: gfanton <[email protected]> Co-authored-by: Manfred Touron <[email protected]>
This PR reverts gnolang#1385 following @jaekwon gnolang#1320 (comment) <!-- please provide a detailed description of the changes made in this pull request. --> <details><summary>Contributors' checklist...</summary> - [ ] Added new tests, or not needed, or not feasible - [ ] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [ ] Updated the official documentation or not needed - [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [ ] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details> Signed-off-by: gfanton <[email protected]>
ref #1320
This draft PR demonstrates a way for fixing flaky BFT tests.
As mentioned in #1320 the main issue is that some channels are not fully consumed, leading to deadlocks. By using buffered channels as proxy in state tests, we can easily avoid these deadlocks. However, I don't think this is the best approach, as these channels were originally set to zero (capacity) for good reasons, but perhaps it could make sense for testing purposes.
In any case, if anyone has a better idea for a solution, your suggestions are welcome.
Contributors' checklist...
BREAKING CHANGE: xxx
message was included in the description