Skip to content
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

Running tests with different feature sets / architectures #63

Closed
sffc opened this issue Apr 28, 2020 · 10 comments · Fixed by #376
Closed

Running tests with different feature sets / architectures #63

sffc opened this issue Apr 28, 2020 · 10 comments · Fixed by #376
Assignees
Labels
C-test-infra Component: Integration test infrastructure T-core Type: Required functionality
Milestone

Comments

@sffc
Copy link
Member

sffc commented Apr 28, 2020

We should think about how we test different feature sets and architectures. By default, cargo test only tests your default architecture and the crate's default features.

Examples of things we want to test:

  • std vs. no_std environment (by enabling or disabling the std feature)
  • Building for the wasm32_unknown_unknown architecture
  • Building on non-Linux, non-x86 architectures like Windows, macOS, ARM

Note: rust-lang/cargo#2911 is a feature request to allow integration tests to choose different feature sets.

@sffc sffc added the C-test-infra Component: Integration test infrastructure label Apr 28, 2020
@zbraniecki
Copy link
Member

You can run cargo test --all-features and make some tests/benchmarks only run when the feature is enabled - I use it here: https://github.com/zbraniecki/unic-locale/blob/master/unic-langid-impl/Cargo.toml#L43

@sffc
Copy link
Member Author

sffc commented May 7, 2020

Thanks; that's a great tip for the command line you need to run in order to invoke cargo test with a certain set of features.

Suppose a crate has three features A, B, and C, and A is on by default. You may want to test all the permutations:

  • (no features)
  • A
  • B
  • C
  • A + B
  • A + C
  • B + C
  • A + B + C

You want to do this because enabling or disabling features could cause the crate to compile or not compile. At the very least, you'd want to test with the std feature on and off.

But, supposing we do test all permutations, we should check for:

  1. Whether it compiles
  2. Whether the tests pass
  3. How the binary size is impacted

The third one is interesting because if you add code that bloats the binary, but only when a feature is specifically enabled, that's a lot less important than if you add code that bloats the binary even with all features disabled.

@sffc sffc added the T-core Type: Required functionality label May 7, 2020
@zbraniecki
Copy link
Member

Features are generally meant to be additive.
I don't think it's common for a package to have features A, B and C that need to be tested in all permutations. Disabling a feature should not cause it to not compile.

@Manishearth has way more experience with it, so I'd be curious on his recommendation for us.

@sffc
Copy link
Member Author

sffc commented May 7, 2020

Disabling a feature should not cause it to not compile.

Disabling the std feature is a great example of one that could cause the library to fail to compile, even if it compiles with the feature enabled. Generally, features that cause code or dependencies to conditionally compile could cause build breakages when those conditional dependencies are removed.

Probably "no features", "default features", and "all features" are sufficient to cover most cases we care about. We should test more permutations only if it's easy.

@Manishearth
Copy link
Member

Yeah, no+default+all is a decent set. If we have "weird" features that change how more than just one component work we might want to add both of their cases explicitly to the matrix. E.g. I can see us running no+all with and without the std feature.

@zbraniecki
Copy link
Member

This may solve it for us - https://github.com/frewsxcv/cargo-all-features

@sffc
Copy link
Member Author

sffc commented May 27, 2020

Nice!

@sffc
Copy link
Member Author

sffc commented Jun 25, 2020

Interesting references: rust-lang/cargo#7507 removed support for both --no-default-features and --all-features in a workspace root, and rust-lang/cargo#7525 added --all-features back again.

@zbraniecki
Copy link
Member

@sffc - this is the last issue blocking Q2 deliverables milestone. Can you update its status?

@sffc
Copy link
Member Author

sffc commented Sep 14, 2020

It will be fixed as soon as @echeran's #172 is checked in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-test-infra Component: Integration test infrastructure T-core Type: Required functionality
Projects
None yet
4 participants