Skip to content

Separate tests from code under test in libraries #12393

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

Closed
milibopp opened this issue Feb 19, 2014 · 10 comments
Closed

Separate tests from code under test in libraries #12393

milibopp opened this issue Feb 19, 2014 · 10 comments

Comments

@milibopp
Copy link
Contributor

The bug reported in #12383 made me aware of the convention that tests in non-std libraries often reside in the same file as the code under test. The privacy issue seen in that bug appears to be an immediate consequence of this convention. The convention also leads to pretty large source files that are imho difficult to handle.

Thus I would like to discuss whether this convention should be changed. To suggest a concrete alternative:

  • Separate the tests into logically coherent units that focus on testing a certain functionality, each of which resides in a separate file
  • Put all these test files into a dedicated test module that is flagged with one #[cfg(test)] in lib.rs; the test module may have substructure depending on the complexity of the library being tested
@emberian
Copy link
Member

I don't think this is necessary, and I think there is value to having tests live next to the unit they are testing. Out-of-crate tests are fine too, we can add those separately in src/test/run-pass.

@dead10ck
Copy link

IMO, the tests should be separate from the code. Oftentimes, the LOC for tests can get larger than the library itself. I like what Go does, where each source file foo.go has its own unit testing code in foo_test.go. go test just looks for these *_test.go files and runs them all against their respective source files. You can do something similar with Rust if you declare a module at the bottom of the source file and define the testing module in its own file, but then you have to import everything you want to test from the parent module. IMHO, this feels very clunky. I should at least be able to separate my tests from the code in my library if I want to.

@steveklabnik
Copy link
Member

/cc @aturon , man of many conventions.

@pcwalton
Copy link
Contributor

@dead10ck I think this is good use case for glob imports; just have the test use module_that_i_am_testing::*. I don't like the way Golang mashes all .go files in the same level of the directory structure into the same namespace.

@steveklabnik
Copy link
Member

Conventions changes go through the RFC process today, and so I'm giving this issue a close. Please open an RFC if you'd like to persue this further. Thanks.

@ofek
Copy link

ofek commented Apr 22, 2017

@steveklabnik Was there ever an RFC? If not, is there now a standard way to separate tests?

@computersarecool
Copy link

@steveklabnik I am also interested in an answer to this.

@milibopp
Copy link
Contributor Author

Seeing this popping up now again in my inbox. I never opened an RFC for this and kind of got used to it by now ;)

@mitermayer
Copy link

I truly believe that giving a better API/Option to write tests outside of the same file would be extremely beneficial for RUST language as whole. It would reduce the barrier for other adopters as well as ensure file size can be kept small without sacrificing on reducing documentation/testing.

I really would love if this issue could be reconsidered.

@ofek
Copy link

ofek commented Feb 12, 2020

Has there been any progress on this (RFC)?

bors added a commit to rust-lang-ci/rust that referenced this issue Jul 25, 2022
internal: Remove `Interned` usage from nameres collector
flip1995 pushed a commit to flip1995/rust that referenced this issue Mar 7, 2024
fix [`derive_partial_eq_without_eq`] FP on trait projection

fixes: rust-lang#9413 rust-lang#9319

---

changelog: fix [`derive_partial_eq_without_eq`] FP on trait projection

Well, this is awkward, it works but I don't understand why, why `clippy_utils::ty::implements_trait` couldn't detects the existance of `Eq` trait, even thought it's obviously present in the derive attribute.
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

No branches or pull requests

8 participants