tests: add nightly test for Protocol Arbitrary impl to assert it covers all the Protocol variants. #76
Conversation
to assert it covers all the Protocol variants.
|
|
||
| // Assert all `Protocol` variants are covered | ||
| // in its `Arbitrary` impl. | ||
| #[cfg(nightly)] |
There was a problem hiding this comment.
Nicely done! At first I thought that we'd need the rust-version proc macro here but cfgs also work very well!
tests/lib.rs
Outdated
| @@ -12,6 +12,8 @@ use std::{ | |||
| str::FromStr, | |||
| }; | |||
|
|
|||
| const IMPL_VARIANT_COUNT: u8 = 32; | |||
There was a problem hiding this comment.
We could make this a const on the type :)
There was a problem hiding this comment.
ehe I wasn't even aware of that feature, thanks Thomas! ❤️
.github/workflows/build.yml
Outdated
| with: | ||
| toolchain: nightly | ||
| command: test | ||
| args: nightly |
There was a problem hiding this comment.
you mean the nightly args? It's just so we filter and only run the tests with nightly on the name. I tried to think of a way to use the #[cfg(nightly)] but couldn't come with a way.
There was a problem hiding this comment.
Ah right. Can't we just run all tests? What is the issue with that? :)
There was a problem hiding this comment.
Just redundancy, do you suggest then that we only run tests once and with nightly?
There was a problem hiding this comment.
I don't think CI time matters much in this repository. In other words, I would assume running tests twice is not an issue.
| - name: Nightly Test | ||
| uses: actions-rs/cargo@v1 | ||
| env: | ||
| RUSTFLAGS: '--cfg nightly -Zcrate-attr=feature(variant_count)' |
There was a problem hiding this comment.
I forgot that we need to activate features, this is 🔥🔥🔥
via associated constant.
| @@ -85,10 +85,14 @@ impl Arbitrary for Ma { | |||
| #[derive(PartialEq, Eq, Clone, Debug)] | |||
| struct Proto(Protocol<'static>); | |||
|
|
|||
| impl Proto { | |||
| const IMPL_VARIANT_COUNT: u8 = 32; | |||
There was a problem hiding this comment.
I was actually thinking of directly on the enum as Protocol::VARIANT_COUNT.
I am sure there are also proc-macros for this that we could enable only during testing if this nightly stuff is too fancy 😁
There was a problem hiding this comment.
but Protocol lives in the crate, the VARIANT_COUNT refers to Proto's arbitrary implementation count. Protocol's count is what we get with mem::variant_count you mean implementing it on Protocol in the tests?
There was a problem hiding this comment.
Right, that would unnecessarily increase the public API. Leave it as is! :)
.github/workflows/build.yml
Outdated
| with: | ||
| toolchain: nightly | ||
| command: test | ||
| args: nightly |
There was a problem hiding this comment.
I don't think CI time matters much in this repository. In other words, I would assume running tests twice is not an issue.
and remove it from the test name
ok thanks, updated! |
This PR follows both @mxinden and @thomaseizinger on #65.
Tried to add multiple toolchains in a single step but apparently it's not possible.
Test running here
Closes #65