Conditionally building rust tests in the "extra" configuration

[stevenengler]

Shadow uses the "extra" configuration to build and run tests that we don't want to build/run by default. For example these include tor-related tests and golang-related tests. We don't want to require users to have tor or golang installed to run tests. We also don't want to add long running tests by default such as the tor tests which can take minutes to run. Finally we don't want to significantly increase the build time of the tests by default. We also would like to minimize the number of dependencies used in the default case to make maintenance easier, but maybe this doesn't mean much since Shadow already has over 100 dependencies.

A question raised by #3242 is "how do we want to build rust tests in this extra configuration?". To only run the tests in the "extra" configuration we can simply add CONFIGURATIONS extra to the add_shadow_tests cmake definition. But to only build the tests in the "extra" configuration, we need to tell Cargo to conditionally build the tests.

Two possibilities might be to use Cargo feature flags or use Cargo examples, as described in (1) and (2) of this SO answer.

1. Use Cargo examples: Put the test's dependencies in [dev-dependencies] and put the test as an [[example]]. Then you can build the examples using cargo build --example. Then we'd need to locate the generated binary and run it from shadow's config like we do with other tests. (Is there a consistent way to get the path to built examples like there is for built binaries?)

2. Use Cargo feature flags: Put the test's dependencies as optional dependencies, then use an "extra" feature to enable them. You would then add the test as a [[bin]] like usual and use required-features = ["extra"] to only build it when using cargo build --features extra. Then we'd run it from shadow's config like we do with other tests.

I don't know if there's any downsides to one approach over the other, or if there are alternatives. The two approaches above seem similar, so maybe there isn't really much to discuss.

[robgjansen]

I've used the option 1 ("examples") pattern before in proteus and I really like it. To me, that pattern more clearly represents the "extra" nature of these tests and would then be less likely to be confused with other features we may someday add to Shadow itself. Both seem to enable what we want technically, so this is just my personal usability preference.

[robgjansen]

Hmm, it seems as if you're right! Paying that extra cost is probably bad enough that we would want to avoid it even if it does make the Cargo.toml slightly less fun (for some of us).

There seems to be a way to avoid building dev deps on nightly:

cargo test -Z avoid-dev-deps

But (1) that's on nightly, and (2) there are going to be deps in the dev-deps that we probably do want to build for the tests, but we just want to avoid some of the deps for the extra tests/examples. I think option 2 (feature flags) still allows for that.

[stevenengler]

For 1, don't all dev-dependencies get pulled in and built when running tests? So we'd end up still paying for them when building tests in the non-extra configuration?

I think so, but I don't think we build tests (as in cargo test) for the test package in src/test/. All of the tests are built as non-test binaries listed as just [[bin]] which wouldn't pull in the dev dependencies.

[robgjansen]

That begs the question: is our current build layout ideal? Or are there significant changes that we would want to make? (For example, I've long had a dream of removing cmake someday:)

[stevenengler]

There's probably a better way we could do tests, but I'm not sure what's best. Maybe something similar to what you did in proteus (but I haven't looked in detail), or maybe with a custom cargo test harness. The tests also need to work well with rust's unit tests, miri tests, and loom tests, since they will all be run under the same cargo test framework. We also have tests written in C and Go so it would need to integrate with those somehow. But this seems like a lot of work. We'd need a good design that works for all existing shadow/linux/tor/golang/examples/etc tests, probably a proof-of-concept, and then to update all the existing tests to use this new design.

[sporksmith]

I think so, but I don't think we build tests (as in cargo test) for the test package in src/test/. All of the tests are built as non-test binaries listed as just [[bin]] which wouldn't pull in the dev dependencies.

Ah right. I suppose going this route would then prevent us from adding unit tests to our test crate (e.g. for helper or utility functions), but maybe that's ok.

I'd still lean slightly towards option 2, but I'm fine with either.