Skip to content

Conversation

@mladedav
Copy link
Collaborator

Motivation

#3485 moved WithRejection behind a feautre gate but this test was not gated.

Why this wasn't caught in that PR's CI

cargo hack checks just lib code, not tests, so the CI did not catch it there. Clippy is run with --all-targets --all-features and tests are run with --all-features so it wasn't caught there either.

Solution

Compile the test only when the required feature is enabled.

We might also want to add a check to CI to catch this but I don't see it as necessary as this only prevents compilation of tests without all features, so users are not affected. However, developers may be surprised when they clone this repository and running cargo test (as opposed to cargo test --all-features) fails.

This does not happen regularly though so I'm not sure if running more stuff in CI is worth it. We could add --all-targets to the cargo hack invocation but that might be problematic in itself because then I think it will be compiled with dev dependencies features as well as normal features. Or we can just also run cargo test and cargo test --no-default-features to test these two scenarios on top of what we're already testing. Or the same thing with cargo check or cargo clippy.

@mladedav mladedav merged commit cc11b5a into main Sep 22, 2025
35 of 36 checks passed
@mladedav mladedav deleted the dm/fix-tests branch September 22, 2025 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants