ZeroTrie: add Bake, Yoke, and ZeroFrom impls #4021
Conversation
| extern crate std; | ||
| test_bake!( | ||
| DeriveTest_ZeroTrie_ZeroVec<'static>, | ||
| crate::DeriveTest_ZeroTrie_ZeroVec { |
There was a problem hiding this comment.
wrapping the zerotrie in a struct doesn't add anything to the test, and it complicates it because it now also requires the struct's bake to be stable. I'd also prefer this as a unit test next to the code under test.
There was a problem hiding this comment.
Please note that these tests are directly based on the ones in utils/zerovec/src/yoke_impls.rs
wrapping the zerotrie in a struct doesn't add anything to the test
It tests that the zerotrie can be used in a struct with derives. Without this test, it's possible that the imps are missing or incomplete. This was a problem in the past which is why we added these tests in the zerovec crate. I think we should err on the side of testing this behavior for any zero-copy data structure.
and it complicates it because it now also requires the struct's bake to be stable
Why? If the bake changes, update the test.
I'd also prefer this as a unit test next to the code under test.
This doesn't catch problems like private fields. Given that this tests external behavior it seems better for this to be in an integration test.
There was a problem hiding this comment.
I agree with Shane here, this is our standard pattern for testing derive bounds for generic stuff
| right: `"zerotrie :: test :: DeriveTest_ZeroTrieSimpleAscii_ZeroVec { _data : zerotrie :: ZeroTrieSimpleAscii { store : zerovec :: ZeroVec :: new () } , }"`', experimental/zerotrie/src/test.rs:46:5 | ||
| */ | ||
| #[test] | ||
| #[ignore] // TODO: Fix (see error message above) |
There was a problem hiding this comment.
we already have ignored bake tests somewhere that are linked to a Rust issue
Toward #2909
Left #4020 for a follow-up