Skip to content

no_expect_outside_tests fires on src/**/tests/*.rs modules declared #[cfg(test)] from a parent file #417

Closed

Description

@leynos

Three repositories on the rolling suite fail their Lint gate on main with no_expect_outside_tests reported inside test modules:

In each case the file lives under a tests/ directory beneath src/ and is pulled in by a #[cfg(test)] mod declaration in a parent file, so the lint's view of the file does not see the cfg(test) attribute and treats it as production code. Axinite hit the same shape on 2026-09-05 with cfg(test) helper functions (#356 made them fallible to satisfy the lint).

Expected: the lint recognises a module as test-only when its declaring mod item carries #[cfg(test)] (or the file sits under a tests/ directory beneath src/), or the suite documents the required layout and the estate's repositories are moved to it.

Found during the mutation-cargo.yml repin sweep (2026-09-05/06); all three lanes were red before the sweep touched them.

Activity

  1. added
    bugSomething isn't working
    enhancementNew feature or request
    highUnblocks delivery within the current sprint or release window. Reaction time is days, not hours.
    on Sep 5, 2026
  2. leynos commented on Sep 6, 2026

    @leynos
    OwnerAuthor

    Closing: this is correct lint behaviour, not a defect.

    House policy, in the addressing-whitaker-findings skill (~/.claude/skills/addressing-whitaker-findings/SKILL.md, under no_expect_outside_tests and no_unwrap_or_else_panic): fixture and helper functions are not tests. A fixture arranges state; arrangement can fail, so it must return Result and propagate errors. Only the test body may unwrap, because there a failure is the test verdict.

    Every site listed here is a helper, not a test body. ddlint's parse_single_item in src/parser/tests/helpers.rs is the clearest case: it is a plain fn that other tests call. The lint reporting it is the suite doing its job, and exempting it would have removed the policy's teeth inside exactly the code the policy is about. #418 is closed unmerged for that reason.

    The expected behaviour proposed above, recognising a module as test-only when its declaring mod item carries #[cfg(test)], would have that effect, so it is not the fix. The fix is in the three repositories: convert the reported helpers to return anyhow::Result or the domain error, with tests consuming via ? or unwrapping in the body. One pull request each, following the skill's playbook and its clippy interactions (panic_in_result_fn denies assert! in Result-returning tests; shadow_reuse fires on let x = x?).

    Two findings from the investigation are worth keeping on the record even though the change is abandoned.

    #[cfg(test)] does not reach a late lint pass as written: the compiler evaluates it during expansion and leaves a parsed CfgTrace attribute, and is_cfg_test_attribute opens by rejecting every parsed attribute. That detection has therefore matched nothing since the attribute became parsed. Under the policy it should not exempt helpers anyway, so whether to remove the dead branch or document it is a separate question.

    Every cfg(test) fixture in the suite is vacuous: each puts the call inside a #[test] function, so it passes on the test attribute rather than on module ancestry, and a fixture not built with --test has its module stripped before the lint runs. Worth knowing for whoever next touches that area.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingenhancementNew feature or requesthighUnblocks delivery within the current sprint or release window. Reaction time is days, not hours.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions