Skip to content

The "testing" section of the rust book needs a small correction #24030

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
lucatrv opened this issue Apr 3, 2015 · 7 comments · Fixed by #24042
Closed

The "testing" section of the rust book needs a small correction #24030

lucatrv opened this issue Apr 3, 2015 · 7 comments · Fixed by #24042

Comments

@lucatrv
Copy link

lucatrv commented Apr 3, 2015

The code example at https://doc.rust-lang.org/book/testing.html#documentation-tests should be corrected changing mod tests { into mod test {.

@steveklabnik
Copy link
Member

nice catch!

@callahad
Copy link
Contributor

callahad commented Apr 4, 2015

Should this instance be mod test, or should everything else be mod tests instead?

Judging from crates.io, it looks like mod tests is more common. Granted, half of it is alexcrichton, but still ;-)

Incoming PR to that effect.

@callahad
Copy link
Contributor

callahad commented Apr 4, 2015

$ grep -r 'mod tests {' src/lib* | wc -l
      63
$ grep -r 'mod test {'  src/lib* | wc -l
      58

Oh, really now?

@lucatrv
Copy link
Author

lucatrv commented Apr 4, 2015

I actually vote for everything else to be mod tests.

@callahad
Copy link
Contributor

callahad commented Apr 4, 2015

Aw, shoot. From some more grepping, the style guide and the language reference both refer to mod test. Going with that.

@lucatrv
Copy link
Author

lucatrv commented Apr 4, 2015

Ok but... to me it doesn't make sense that we use mod test and then we name the folder tests, as they both can contain multiple tests...

@callahad
Copy link
Contributor

callahad commented Apr 4, 2015

/me considers giving Steve a choose-your-own-adventure pair of pull requests.

callahad added a commit to callahad/rust that referenced this issue Apr 4, 2015
Fixes rust-lang#24030

Of the four code samples with modules in TRPL:

    - 2 use `mod test`
    - 2 use `mod tests`

We should be consistent here, but which is right? The stdlib is split:

    $ grep -r 'mod tests {' src/lib* | wc -l
          63
    $ grep -r 'mod test {'  src/lib* | wc -l
          58

Subjectively, I like the plural. Maybe you, dear reviewer, do too?
callahad added a commit to callahad/rust that referenced this issue Apr 4, 2015
Fixes rust-lang#24030

Of the four code samples with modules in TRPL:

    - 2 use `mod test`
    - 2 use `mod tests`

We should be consistent here, but which is right? The stdlib is split:

    $ grep -r 'mod tests {' src/lib* | wc -l
          63
    $ grep -r 'mod test {'  src/lib* | wc -l
          58

Subjectively, I like the plural, but both the language reference and the
style guide recommend the singular. So we'll go with that here, for now.
Manishearth added a commit to Manishearth/rust that referenced this issue Apr 7, 2015
 Fixes rust-lang#24030

Of the four code samples with modules in TRPL:

    - 2 use `mod test`
    - 2 use `mod tests`

We should be consistent here, but which is right? The stdlib is split:

    $ grep -r 'mod tests {' src/lib* | wc -l
          63
    $ grep -r 'mod test {'  src/lib* | wc -l
          58

Subjectively, I like the plural, but both the language reference and the
style guide recommend the singular. So we'll go with that here, for now.

r? @steveklabnik
bors added a commit that referenced this issue Apr 25, 2015
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
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

Successfully merging a pull request may close this issue.

3 participants