Skip to content

regression: error: module should be marked with the #[cfg(test)] attribute #140225

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
cuviper opened this issue Apr 23, 2025 · 6 comments
Closed
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-cfg Area: `cfg` conditional compilation A-proc-macros Area: Procedural macros C-bug Category: This is a bug. regression-from-stable-to-beta Performance or correctness regression from stable to beta. S-has-bisection Status: A bisection has been found for this issue S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue
Milestone

Comments

@cuviper
Copy link
Member

cuviper commented Apr 23, 2025

[INFO] [stdout] error: module should be marked with the `#[cfg(test)]` attribute
[INFO] [stdout]        
[INFO] [stdout]          = help: add `#[cfg(test)]` to the module
[INFO] [stdout]        
[INFO] [stdout]   --> crates/after-test_test/src/lib.rs:3:1
[INFO] [stdout]    |
[INFO] [stdout] 3  | / mod panics {
[INFO] [stdout] 4  | |     fn panic_with_message() {
[INFO] [stdout] 5  | |         panic!("This is a panic message");
[INFO] [stdout] ...  |
[INFO] [stdout] 10 | |     fn test_should_panic() {}
[INFO] [stdout] 11 | | }
[INFO] [stdout]    | |_^

(and several more like that)

That particular error message comes from their own proc-macro. The module does have the expected attribute, but I suppose something must have changed in how we provide that to the macro.

#[after_test::cleanup(panic_with_message)]
#[cfg(test)]
mod panics {
    fn panic_with_message() {
        panic!("This is a panic message");
    }

    #[test]
    #[should_panic(expected = "This is a panic message")]
    fn test_should_panic() {}
}

Version it worked on

It most recently worked on: 1.86.0

Version with regression

rustc 1.87.0-beta.5 (386abeb93 2025-04-19) in crater #139827.

@rustbot modify labels: +regression-from-stable-to-beta -regression-untriaged

@cuviper cuviper added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Apr 23, 2025
@cuviper cuviper added this to the 1.87.0 milestone Apr 23, 2025
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-untriaged Untriaged performance or correctness regression. labels Apr 23, 2025
@cyrgani
Copy link
Contributor

cyrgani commented Apr 23, 2025

cargo test with the following reduced macro:

use proc_macro::TokenStream;

#[proc_macro_attribute]
pub fn cleanup(_: TokenStream, item: TokenStream) -> TokenStream {
    dbg!(&item);
    TokenStream::new()
}

and usage

#[y::cleanup]
#[cfg(test)]
mod panics {}

beta is indeed just printing

[src/lib.rs:5:5] &item = TokenStream [
    Ident {
        ident: "mod",
        span: #0 bytes(41..44),
    },
    Ident {
        ident: "panics",
        span: #0 bytes(45..51),
    },
    Group {
        delimiter: Brace,
        stream: TokenStream [],
        span: #0 bytes(52..54),
    },
]

but stable gets

[src/lib.rs:5:5] &item = TokenStream [
    Punct {
        ch: '#',
        spacing: Alone,
        span: #0 bytes(28..29),
    },
    Group {
        delimiter: Bracket,
        stream: TokenStream [
            Ident {
                ident: "cfg",
                span: #0 bytes(30..33),
            },
            Group {
                delimiter: Parenthesis,
                stream: TokenStream [
                    Ident {
                        ident: "test",
                        span: #0 bytes(34..38),
                    },
                ],
                span: #0 bytes(33..39),
            },
        ],
        span: #0 bytes(29..40),
    },
    Ident {
        ident: "mod",
        span: #0 bytes(41..44),
    },
    Ident {
        ident: "panics",
        span: #0 bytes(45..51),
    },
    Group {
        delimiter: Brace,
        stream: TokenStream [],
        span: #0 bytes(52..54),
    },
]

@cyrgani
Copy link
Contributor

cyrgani commented Apr 23, 2025

Regression in rust-lang-ci@8d60e92
The PR introducing the regression in this rollup is #138844: expand: Leave traces when expanding cfg attributes

searched nightlies: from nightly-2025-01-01 to nightly-2025-04-23
regressed nightly: nightly-2025-03-28
searched commit range: a2e6356...3f55023
regressed commit: 3f55023

bisected with cargo-bisect-rustc v0.6.9

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start=2025-01-01 -- test 

@rustbot label S-has-mcve S-has-bisection

@rustbot rustbot added S-has-bisection Status: A bisection has been found for this issue S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue labels Apr 23, 2025
@jieyouxu
Copy link
Member

jieyouxu commented Apr 24, 2025

This needs a crater run, because it changes observable behavior (in an intended way) - proc macros can no longer see expanded cfg(true) attributes.

cc @petrochenkov: can you please double-check if this is intended observable behavior change re. #138844.

@jieyouxu jieyouxu added A-attributes Area: Attributes (`#[…]`, `#![…]`) A-proc-macros Area: Procedural macros A-cfg Area: `cfg` conditional compilation and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Apr 24, 2025
@jieyouxu
Copy link
Member

I also tagged #138844 w/ compat relnotes (#140243) to be safe.

@petrochenkov
Copy link
Contributor

This needs a crater run, because it changes observable behavior (in an intended way) - proc macros can no longer see expanded cfg(true) attributes.

cc @petrochenkov: can you please double-check if this is intended observable behavior change re. #138844.

Yes, this is an intended change.

@jieyouxu
Copy link
Member

jieyouxu commented May 1, 2025

Closing in favor of compat relnotes (#140243) since this is an intended change.

@jieyouxu jieyouxu closed this as completed May 1, 2025
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 2, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-cfg Area: `cfg` conditional compilation A-proc-macros Area: Procedural macros C-bug Category: This is a bug. regression-from-stable-to-beta Performance or correctness regression from stable to beta. S-has-bisection Status: A bisection has been found for this issue S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue
Projects
None yet
Development

No branches or pull requests

6 participants