-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Rename tests modules to test to reflect guidelines. #23870
Conversation
(Except for the ones in std::old_io, librbml, librustdoc/html/markdown.rs and libtest, as they already expose a public module test or use libtest or another module named test.)
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @huonw (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see CONTRIBUTING.md for more information. |
Thanks for the PR! We may want a more concrete convention, however, before going through and changing all the modules. I believe many of the guidelines haven't necessarily come under high scrutiny just yet, and we may want to hold off on this change until there's some more consensus around whether the name |
☔ The latest upstream changes (presumably #23549) made this pull request unmergeable. Please resolve the merge conflicts. |
+1 to module `tests`: it usually contains multiple test functions. See
precedent 'Examples' in docs. And avoid name conflicting with 'test' where
libtest is imported.
|
r? @alexcrichton (transferring reviewership, don't have the bandwidth right now.) |
Ok at this point I think I'm going to close this while discussion happens about what the convention should be here, if any. Would you mind opening up a discuss post on this topic to gauge other opinions? Thanks! |
Changes the style guidelines regarding unit tests to recommend using a sub-module named "tests" instead of "test" for unit tests as "test" might clash with imports of libtest (see #23870, #24030 and http://users.rust-lang.org/t/guidelines-naming-of-unit-test-module/1078 for previous discussions). r? @alexcrichton
Follow-up of #23839: Rename another occurence of "mod tests" in documentation as well and rename unit test modules in the repository accordingly to reflect the guidelines.
However, I had to leave the example for the benchmark tests as it is because importing libtest clashes with "mod test". The same applies for the "tests" submodules in libstd/old_io/mod.rs, librbml/lib.rs, librustdoc/html/markdown.rs and libtest/lib.rs, as they already expose a public module test or use libtest or another module named test.
Which makes me wonder if the convention to place unit tests in a submodule named "test" is the right choice as it is impossible to do so when importing libtest at the same time (renaming libtest is another option).