tests(app): 💎 polish mock consensus test infrastructure #4185
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 a previous PR review (_i could not find a link, i'm afraid_) @hdevalence suggested that we remove the `common::start_test_node()` helper. this belatedly performs that code transformation. Co-authored-by: Henry de Valence <hdevalence@penumbralabs.xyz>
now that we have outlined the distinct components of shared test infrastructure, this import block is no longer load-bearing.
this was useful in early development, but we can (a) break this into distinct test files, and (b) get rid of `mock_consensus_can_send_an_init_chain_request` which is now obselete thanks to a more sophisticated test suite.
cratelyn
force-pushed
the
kate/mock-consensus-polish-pass-pt-ii
branch
from
8b841db to
f422c60
Compare
conorsch
approved these changes
Apr 11, 2024
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.
see #3588. follows #4184 and #4181.
this takes a pass through the shared, Penumbra-specific test infrastructure for mock consensus tests. notably, this decomposes
init_chain.rs, which has now become somewhat redundant with the existence of other more involved tests of e.g. validator uptime tracking.this also cleans up some unused imports, guards against future occurrences of that issue (sharing code in
tests/files is awkward), and decomposes thecommon/mod.rsfile into some distinct standalone components.this also belatedly removes the
common::start_test_node()helper. at some point (i was unable to find the link) it was suggested that we refrain from a shared setup helper like that. this branch removes that helper, and updates its call-sites.this branch is largely code motion, and is intended to be a last bit of cleanup as we prepare for #3588 to wind down. ❤️
checklist before requesting a review
If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: