chore: allow crater runs #8171
chore: allow crater runs #8171
Conversation
Copies the Ed25519Verifier code directly to tower-batch-control tests
bishopcheckmate
requested review from
arya2 and
upbqdn
and removed request for
a team
github-actions
bot
added
the
C-trivial
The timeouts copied from tower-batch-control seems too low and there must have been a reasoning why were they introduced
There was a problem hiding this comment.
This looks great, thank you!
I left an optional suggestion for removing the dependency on zebra-chain and metrics.
Make sure the tests that are on by default:
run reasonably fast
don't use too much network
(check crater docs for any other requirements)
I didn't find any guidelines either, I'm not sure if there are any constraints in the code, or if it's manually observed and configured on a per-project basis here: https://github.com/rust-lang/crater/blob/master/config.toml
arya2
added
C-enhancement
github-actions
bot
added
the
C-trivial
upbqdn
removed
the
C-trivial
github-actions
bot
added
the
C-trivial
|
The CI failure seems unrelated to the changes, we should try to fix that or merge this manually. |
|
@bishopcheckmate just FYI: your code is approved to be part of the zebra main branch but we are having CI issues with external pull requests. Right now, @gustavovalverde is working on fixing this issue. We will keep you posted, thanks for your contribution. I hope this problem do not discourage you to submit more code. |
|
hey, no problem at all. I'll definitely keep contributing here and there, just been busy recently |
gustavovalverde
merged commit d1d128a
into
ZcashFoundation:main
* chore(dev-deps): always specify the version for dev-dependencies * test(tower-batch-control): remove zebra-consensus dev dependency Copies the Ed25519Verifier code directly to tower-batch-control tests * test(zebra-consensus): update ed25519 verifier tests with ones from batch-control * test(zebra-consensus): restore previous timeout values The timeouts copied from tower-batch-control seems too low and there must have been a reasoning why were they introduced * chore: update dev-deps versions to beta.33 * chore(tower-batch-control): remove dev-dependency on metrics * chore(tower-batch-control): remove zebra-chain dev-dependency * Update zebra-scan/Cargo.toml * bump all versions to match current release * fix missed commas in version bumps --------- Co-authored-by: Arya <[email protected]> Co-authored-by: Alfredo Garcia <[email protected]> Co-authored-by: Pili Guerra <[email protected]>
Motivation
Fixes #6924
PR Author Checklist
Check before marking the PR as ready for review:
For significant changes:
Specifications
This change adds versions to all dev-dependencies so that they end up being packed together with crates on crates.io thus allowing crater runs.
It also removes the circular dependency between
tower-batch-controlandzebra-consensusby copying the Ed25519 verifier code into batch-control tests. Also updates the verifier tests in consensus to their state from tower-batch-control.There are still unadressed points in #6924:
It's hard for me to define reasonably fast and too much networking. I can try to measure those on my PC if you want.
Regarding crater docs for any other requirements, I couldn't find any guidelines on that.
Testing
This should be tested using
cargo publish, however it's not currently possible due to git dependencies added in #8136.Reviewer Checklist
Check before approving the PR:
PR blockers can be dealt with in new tickets or PRs.
And check the PR Author checklist is complete.