-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: verifreg: add initial cbor encoding forms tests #263
Conversation
Rust version: filecoin-project/builtin-actors#1538 |
cba8a34
to
e744dea
Compare
Starting with ClaimAllocationsParams for now, matching with Rust versions of this.
e744dea
to
bf4c8cd
Compare
removed diagnostic output, the more I look at it the more wasteful it seems when we expand the number of these |
bf4c8cd
to
77ba903
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be overkill to do this for multiple versions of a type when nothing changes.
If we could remove these from next to the code and put them in some place where all versions are accessible we could maybe only update tests when the actual types change. That would reduce code volume a lot for the same value. Idk though it's pretty cheap to copy paste a test and carry all these along.
Regarding duplication - my theory here is that as we start to accumulate these, a pattern should emerge that can be extracted to make this simpler. Perhaps reviving /~https://github.com/filecoin-project/test-vectors at some point for these would be good and then run the tests from both ends. For IPLD I set up this: /~https://github.com/ipld/codec-fixtures which runs a set of fixtures across the latest codec implementations of Go, JS and Rust each night and complains if there are problems. I think a similar pattern could work here, but I don't really want to dive in too deep right now. |
Since I was touching type changes in #262 I thought I'd do an initial version of something I've proposed doing a more comprehensive form of: cbor encoding and round-trip testing of publicly exported types, which is particularly important for the types we pass through builtin-actors. So for now I'm just starting with ClaimAllocationsParams, and am following this up with a PR to builtin-actors with identical fixtures.
Ideally we'd expand this, perhaps slowly, to cover the full suite of types that pass through this barrier, and in the process provide fixtures for other implementations to test against as well.
The diagnostic output might be overkill, I can remove that if someone wants to object, I just like being absolutely clear.