Skip to content

fix: test reliability patches#1954

Closed
faddat wants to merge 92 commits intocometbft:mainfrom
faddat:faddat/add-patches-to-master
Closed

fix: test reliability patches#1954
faddat wants to merge 92 commits intocometbft:mainfrom
faddat:faddat/add-patches-to-master

Conversation

@faddat
Copy link
Contributor

@faddat faddat commented Jan 2, 2024

This PR fixes

  • flakiness in testwriter
  • flakiness in testbyzantineprevoteequivocation

@melekes - I'd like to merge this branch, and then open another torture branch and run the tests a few thousand times. But I think we are in good shape.


NOTES FOR REVIEWERS

Some of the fixes here involved small changes to the tolerances of tests in situations where the code may behave in a slightly variable way without malfunctioning. I bet that is the key item for review.

This needs to be broken into pieces.

I consider this branch to be mergable now, and I suppose what I'll do is make another one where the other tests I identified as having issues are run 100 times. That was totally key to fixing TestTransportMultiplexAcceptNonBlocking because it got me the data I needed to understand the problem.


PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments

@faddat faddat requested a review from a team as a code owner January 2, 2024 21:03
@faddat faddat requested a review from a team January 2, 2024 21:03
@melekes
Copy link
Collaborator

melekes commented Jan 3, 2024

TestByzantinePrevoteEquivocation and TestTransportMultiplexAcceptNonBlocking are still failing.

--- FAIL: TestByzantinePrevoteEquivocation (30.08s)
    byzantine_test.go:292: Evidence from each validator: [<nil> <nil> <nil> <nil>]
    byzantine_test.go:293: Timed out waiting for validators to commit evidence

--- FAIL: TestTransportMultiplexAcceptNonBlocking (0.04s)
    transport_test.go:360: have {{9 11 0} dc28ea6b630e74a67256457f901d9d183da919d7 127.0.0.1:38953 testing 1.2.3-rc0-deadbeef 01 slow_peer {on 127.0.0.1:44371}}, want {{9 11 0} 6[123](https://github.com/cometbft/cometbft/actions/runs/7390672952/job/20106075991?pr=1954#step:5:124)f7e124d306734abc6a4d438a5a734f322dcc 127.0.0.1:37183 testing 1.2.3-rc0-deadbeef 01 fastnode {on 127.0.0.1:35069}}

@faddat
Copy link
Contributor Author

faddat commented Jan 3, 2024

Working on it sir :D

@faddat faddat marked this pull request as draft January 16, 2024 18:09
@faddat
Copy link
Contributor Author

faddat commented Jan 16, 2024

This branch is no longer mergeable and I should break it into individual changes.

github-merge-queue bot pushed a commit that referenced this pull request Feb 6, 2024
This PR addresses test reliability issues in testreader and testwriter. 

From: #1954 


---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [ ] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

---------

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
mergify bot pushed a commit that referenced this pull request Feb 6, 2024
This PR addresses test reliability issues in testreader and testwriter.

From: #1954

---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [ ] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

---------

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
(cherry picked from commit 075d3b5)

# Conflicts:
#	internal/flowrate/io_test.go
melekes added a commit that referenced this pull request Feb 8, 2024
This PR addresses test reliability issues in testreader and testwriter. 

From: #1954 


---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [ ] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

---------

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale For use by stalebot label Feb 11, 2024
github-merge-queue bot pushed a commit that referenced this pull request Feb 12, 2024
This PR should resolve the odd case where a group of rpc tests fail
simultaneously.


from: #1954 


---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [ ] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
mergify bot pushed a commit that referenced this pull request Feb 12, 2024
This PR should resolve the odd case where a group of rpc tests fail
simultaneously.

from: #1954

---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [ ] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

(cherry picked from commit 630a94b)

# Conflicts:
#	rpc/jsonrpc/server/http_server_test.go
@github-actions github-actions bot closed this Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale For use by stalebot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants