search

oxidecomputer / amd-apcb

AMD Generic Encapsulated Software Architecture Platform Security Processor Configuration Block manipulation library
Mozilla Public License 2.0
13 stars 1 forks source link

Add Makefile (for tests, mostly) #37

Closed daym closed 2 years ago

daym commented 2 years ago

Add a Makefile so tests can be run more easily.

daym commented 2 years ago

It would be nice to also test the serde_yaml case somehow.

Background:

serde_yaml has a limitation such that from_str cannot borrow parts of the original string in the returned struct. That means that all the structures in question of amd-apcb have to implement DeserializeOwned, which means none of them are allowed to have lifetime parameters. If they had, the Rust compiler would emit a borrow checker error.

We did modify amd-apcb in this way recently, so it works now. However, it can potentially be silently broken by future changes to amd-apcb since the property is only ensured by convention.

Nice would to be to have a unit test. But obviously, we don't want amd-apcb itself to hard-depend on serde_yaml for such a test, and Cargo.toml dev-dependencies can't be optional.

How could we make buildomat ensure that this property is upheld?

jclulow commented 2 years ago

I see the # TODO: test-compile let _foo: Apcb = serde_yaml::from_str("") comment in the Makefile...

Does this mean: there should be a program that makes an Apcb via from_str to make sure it can compile?

Could you use a Cargo example program to do this, and then build it in a buildomat job with, say, cargo build --example fromyaml?

daym commented 2 years ago

Good idea!

I added it... aaaand it fails.

daym commented 2 years ago

Gan and I fixed it.

Please review :)

daym commented 2 years ago

Note: I tried making serde_yaml optional like described on https://old.reddit.com/r/rust/comments/qw6p9h/dependencies_for_examples/hl0wgzr/ but couldn't make it work. So now, serde_yaml is a mandatory dev-dependency.