test: improve slow tests #3487
test: improve slow tests #3487
Conversation
Codecov Report
... and 7 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more.
|
onbjerg
force-pushed
the
onbjerg/slow-tests
branch
from
750d307
to
b3938b9
Compare
onbjerg
requested review from
rkrasiuk,
mattsse,
Rjected and
gakonst
as code owners
| @@ -68,11 +68,17 @@ impl From<Vec<H256>> for GetBlockBodies { | |||
|
|
|||
| /// The response to [`GetBlockBodies`], containing the block bodies that the peer knows about if | |||
| /// any were found. | |||
| #[derive_arbitrary(rlp, 1)] | |||
| #[derive_arbitrary(rlp, 16)] | |||
There was a problem hiding this comment.
what does the number represent in this derive macro?
There was a problem hiding this comment.
the number of test cases, default is 256, but we set it to 1 previously because it was expensive. with these changes we can bump it to 16, and while still relatively costly, this is manageable. imo better to have more test cases than a single large test case
| #[cfg_attr( | ||
| any(test, feature = "arbitrary"), | ||
| proptest( | ||
| strategy = "proptest::collection::vec(proptest::arbitrary::any::<BlockBody>(), 0..=20)" | ||
| ) | ||
| )] |
There was a problem hiding this comment.
I don't really understand how proptest works,
why does this need to be specified at all?
I would expect that these things would be configured when the proptest is created, or is this how we pass this information to the macro?
this looks a bit weird at first but I don't really care as long as this speeds up tests
There was a problem hiding this comment.
the issue is that we are deriving proptest::Arbitrary instead of specifying how it should create these types, so it's very opaque. by default, all vecs have a range of 0..100, so this type in particular by default will have 0..100 block bodies, each with 0..100 ommers, 0..100 transactions (which can have 0..100 access list items, which can have 0..100 storage slots in each item) and so on. as you can see, this quickly explodes.
since the test is auto-generated, we can't configure this in there.
There was a problem hiding this comment.
a bit ugly :(, but necessary
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
BlockBodies(eth wire)Receipts(eth wire) to 50, each with 5 receipts(All of these are just for
proptesttests)The slowest part is now the build, which can be addressed separately.
Closes #3409