Closed connormullett closed 1 year ago
A few changes are needed:
#[cfg(test)]
guard means it won't be usable outside this crate. The implementation literally only exists when libipld-core
is being tested.quickcheck_macros
is not used.Gen.size
.rust-cid
will soon support quickcheck-1
. There should be a note in the code saying that Cid::arbitrary(g)
should be used when it is available.Thanks for pushing this! Quickcheck tests can be a real life-saver, or a productivity boost, at least for me — but this second part crucially depends on implementing shrink
to quickly find the culprit. Would you be up for adding that? (This is mainly necessary for variably sized inputs that can be large, in this case List
and Map
(and possibly String
and Bytes
; my implementation recipe is to iterate over all ways in which the input can be made “smaller by one”).
Sure! I'll see what I can do
I've been spinning my wheels on this for a while now and haven't made much progress at all. I believe the conflict is because of the signature in shrink
and IPLD
being an enum with IpldIter
being a separate type. I can't figure it out and the documentation basically doesn't exist. I understand why it would be great to have (and the docs do mention how useful it is) but I can't find anything to help implement this
The CI workflow error looks to be not on my side, the github action doesn't have the wasm32 target
I’d try it with something like the following:
fn shrink(&self) -> Box<dyn Iterator<Item = Self>> {
match self {
Ipld::List(list) => {
let list = list.clone();
let first = list.get(0).cloned();
Box::new(
(0..list.len())
.map(move |remove| {
Ipld::List(
list.iter()
.enumerate()
.filter_map(|(idx, ipld)| {
if idx == remove {
None
} else {
Some(ipld.clone())
}
})
.collect(),
)
})
.chain(
first
.map(|ipld| ipld.shrink())
.unwrap_or_else(|| Box::new(std::iter::empty())),
),
)
}
Ipld::Map(_) => todo!(),
_ => Box::new(std::iter::empty()),
}
}
As far as I understand quickcheck, we only need to shrink by one step at a time, we don’t need an iterator that produces all shrunk versions in one go.
PR #167 will incorporate the comments and suggestions from this PR.
This PR can be closed.
ChainSafe's project Forest recently added property testing for serialization/deserialization of datatypes after finding a typo one day. We've been using
quickcheck
and need to implement Arbitrary on some of these types. In order to clean up our codebase from using Ipld wrappers, we'd like to push them upstream into the respective repositories.The tracking issue can be found here
Thanks!