Conversation
cason
left a comment
There was a problem hiding this comment.
For me is not really clear what those tests are actually testing.
Apparently, we ask a node for its status, to know its last block. Then use private GRPC to get this block and check that everything is ok.
This is flaky, I assume, because the node status gets outdated pretty fast (in particular on GitHub cloud/CI) and we at the same time prune old heights.
The solution is to use the private GRPC to test GRPC looks not ideal to me, honestly. But I don't think we want to have all those errors and this solution probably works.
Suggestion: add comments to the code showing the problem of the original version and/or create an issue to define the right way to test what we want to test here (which is also not clear due to the lack of comments).
|
tests are passing now https://github.com/cometbft/cometbft/actions/runs/9554601144 I will go ahead and merge this to prevent more nightly failures. @cason and @melekes if you feel this tests should be changed or modified please let me know. |
This PR changes some gRPC test on nightly. Instead of using a mix of RPC client (to fetch the height) and gRPC to fetch the block and results, the new logic leverages the gRPC get latest height #### PR checklist - [X] 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 9bcafd9)
This PR changes some gRPC test on nightly. Instead of using a mix of RPC client (to fetch the height) and gRPC to fetch the block and results, the new logic leverages the gRPC get latest height #### PR checklist - [X] 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 <hr>This is an automatic backport of pull request #3262 done by [Mergify](https://mergify.com). Co-authored-by: Andy Nogueira <me@andynogueira.dev>
This PR changes some gRPC test on nightly. Instead of using a mix of RPC client (to fetch the height) and gRPC to fetch the block and results, the new logic leverages the gRPC get latest height #### PR checklist - [X] 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 <hr>This is an automatic backport of pull request #3262 done by [Mergify](https://mergify.com). Co-authored-by: Andy Nogueira <me@andynogueira.dev>
This PR changes some gRPC test on nightly. Instead of using a mix of RPC client (to fetch the height) and gRPC to fetch the block and results, the new logic leverages the gRPC get latest height
PR checklist
.changelog(we use unclog to manage our changelog)docs/orspec/) and code comments