-
Notifications
You must be signed in to change notification settings - Fork 13k
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
#[test] visibility changes can produce conflicts and type errors with glob use. #52557
Comments
I propose that instead of expanding: #[test]
fn foo() {} to: pub fn foo(){}
pub mod __test_reexports {
pub use super::foo;
} we expand to: pub mod __test_reexports {
use super::*;
pub fn foo() {...}
} This is a breaking change but only for the semi-pathological case where you have test functions referencing each other. It also brings test into line with standard builds in that |
I bet that's not uncommon enough to make a breaking change. However, I would expect test functions that reference each other to do so via relative paths, and this would only be breaking if absolute paths were used (iiuc). In any case we should probably just fix this and do a Crater run. |
cc @petrochenkov and @rust-lang/compiler We might be able to hack something in name resolution? Perhaps glob imports don't treat test functions as public? Or we give them some weird hygiene marker or something? |
Actually if we did: use __test_reexports::*;
pub mod __test_reexports {
use super::*;
pub fn foo() {...}
} I think we'd be compatible and safe |
Well, technically we should |
I much better prefer rust-lang/rfcs#2471 (comment) as a solution to all our testing woes. I'll maybe try to prototype it soon. |
Reexport tests without polluting namespaces This should fix issue #52557. Basically now we gensym a new name for the test function and reexport that. That way the test function's reexport name can't conflict because it was impossible for the test author to write it down. We then use a `use` statement to expose the original name using the original visibility.
Fixed by #52890 |
Because
#[test]
marks tests as public so that they can be reexported (avoiding E0364), they can cause namespace pollutions that only occur in test builds.Minimal repro:
In a normal build, the only
foo
in scope isB::foo
, but in a test buildA::foo
will shadow it. And produce the following error:The text was updated successfully, but these errors were encountered: