new chapter: testing #288

steveklabnik · 2016-10-07T19:17:05Z

I took some content from the old testing (and documentation) chapters, but also did a lot of re-writing/editing. Whatcha think @carols10cents ?

steveklabnik · 2016-10-07T19:17:34Z

(Added to https://github.com/rust-lang/book/projects/1 as well)

carols10cents · 2016-10-10T20:08:04Z

src/chXX-00-testing.md

+Rust is a programming language that cares a lot about correctness. But
+correctness is a complex topic, and isn't exactly easy to get right. Rust
+places a lot of weight on its type system to help ensure that our programs do
+what we intend, but it cannot help with everything. As such, Rust also includes


Namely, Rust can't figure out that our logic is doing what the users of our software need it to do, so testing those cases is still very important! This tweet feels relevant: https://twitter.com/WAWilsonIV/status/784891724866785280

this tweet has since been deleted

I'm not sure how to concretely improve this here, so if you have suggestions I'm up for it 😄

ugggghhhhhhh why didn't i copy paste the tweet....

I think I'd provide an example here, something like:

We can write a function called add_one, and it can have a signature of accepting an integer as an argument and returning an integer. We can implement that function, and Rust can do all the type checking and borrow checking that we've seen it's capable of doing. What Rust can't check for us is that we've implemented this function as arg + 1 and not arg + 10 or arg - 50! That's where tests come in: we can write a test that passes 3 to our function and checks that we get 4 back, and we can run this test whenever we make changes to our code to make sure we didn't break anything that we knew was previously working.

carols10cents · 2016-10-10T20:10:56Z

src/chXX-00-testing.md

+test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured
+```
+
+## Inverting failure with `should_panic`


Could we show an example of a test that asserts something about an actual function, maybe with assert_eq!, before we introduce should_panic?

So to me, the idea of the structure was "show how tests work, then show all the different kinds of assertions." This would mix that up a bit. Maybe that's just a bad organization though?

Hmm, I think maybe +1 @carols10cents in the sense of "show me how to do testing, then show me what's going on" would be my pref.

carols10cents · 2016-10-10T20:12:15Z

src/chXX-00-testing.md

+
+## Testing equality
+
+Rust provides a pair of macros, `assert_eq!` and `assert_ne!`, that compares


Yeah basically I think the "Testing equality" and "Inverting failure with should_panic" sections should be swapped: i test for equality waaaayyy more than i use should_panic.

here i find my past self disagreeing with my present self. sigh.

👍 for swapping

here i find my past self disagreeing with my present self. sigh.

what does your present self think?

carols10cents · 2016-10-10T20:15:01Z

src/chXX-00-testing.md

+fn it_works() {
+    assert_eq!(4, add_two(2));
+}
+```


Can we show what a failure message for assert_eq! looks like, and show that it doesn't matter which order the arguments go in, really?

carols10cents · 2016-10-10T20:18:12Z

src/chXX-02-integration-testing.md

+
+Because `common.rs` is in our `tests` directory, Cargo is also compiling it as
+its own crate. Because `common.rs` is so simple, we didn't get an error, but
+with more complex code, this might not work. So what can we do?


Wait but... if we have a problem in common/mod.rs, cargo test will still fail, right? Why is this better exactly? Is this a thing that has confused people?

This is something we get regular questions about in #rust and #rust-beginners, yeah.

The key is stuff like extern crate, if it's interpreted as its own crate, then it won't have stuff from extern crate foo in it, but if it's just a submodule, it will have it from the actual parent crate. make sense?

But this section doesn't say anything about extern crate anywhere, just that you'll get 4 sections of test output instead of 3 if you use common.rs instead of mod/common.rs... I guess I'm not seeing what this text is saying might not work where.

Yes, I will clarify it.

I'm still confused about integration test files and crates and the relationship thereof :(

The beginning of the integration test section says:

If you make [a tests directory], and put .rs files inside, Cargo will compile each of [the .rs files] as an individual crate.

and:

We now have an extern crate adder at the top. This is because each test in the tests directory is
an entirely separate crate, and so we need to import our library.

I read this as "each file in tests is compiled into a crate that is independent of each of the other files in tests.

However, in this section, we're demonstrating that if you extract a module into tests/common.rs, because that's a .rs file in the tests directory too, common will be compiled into a crate and get its own section in the tests output.

But we were able to use common::helper() in tests/integration_test.rs without having to say extern crate helper, so does that mean the separate crates in tests are compiled in... like... the same context? Like they all have each other imported as extern crate would already? I'm going to research this too but wanted to confirm with you....

Ok jake has helped me to understand that it's compiling tests/common.rs as a separate crate AND as a module of tests/integration_test.rs because of the mod common..... I'm still struggling with this because we say it's a problem but we don't demonstrate any problems.

I found one example of someone doing this in IRC, but the problem is slightly different than the problem presented here: https://botbot.me/mozilla/rust/2016-03-02/?msg=61394193&page=20

They want to make one tests/lib.rs and then include other test files as mods in there, as well as have helper mods declared in tests/lib.rs and usable in the other test file mods, rather than what's presented here, which is having multiple top-level crates that could all havemod common and work just fine....

carols10cents · 2016-10-10T20:18:44Z

src/chXX-01-testing.md

+
+In this scenario, we have a non-`pub` function, `internal_adder`. Because tests
+are just Rust code, and the `tests` module is just another module, we can
+import and call `internal_adder` in a test just fine.


Wdyt about showing how you can run just one test by specifying the test name to cargo test?

carols10cents · 2016-10-10T20:21:04Z

src/chXX-03-documentation-tests.md

+examples that don't actually work, because the code has changed since the
+documentation has been written. To this end, Rust supports automatically
+running examples in your documentation for library crates. Here's a fleshed-out
+`src/lib.rs` with examples:


We haven't actually talked about doc comments yet, if this chapter is going to come before the "creating a library" chapter.... could we move this section to the doc comments section in that chapter and reference this chapter?

I thought we had it in http://rust-lang.github.io/book/ch03-04-comments.html. This kind of split is something that always confuses some people, they look for advice about testing in the testing chapter, not the documentation chapter. But some people look in the documentation chapter...

We cut doc comments from 03-04 since they only really make sense if you're creating a library and using cargo doc.

carols10cents · 2016-10-10T20:39:03Z

also travis caught some /home/steves you've got in there <3 <3 <3

steveklabnik · 2016-10-10T21:06:15Z

also travis caught some /home/steves you've got in there <3 <3 <3

Yay travis! I tried real hard to not let it happen, ugh. This is why we lint 🎊

steveklabnik · 2016-11-08T21:19:13Z

Okay, resolved everything except this ordering bit.

@steveklabnik

The narrative flows better if we follow what @steveklabnik is doing in rust-lang/book#288. Therefore, I completely copied it.

sophiajt · 2016-11-11T20:17:06Z

src/chXX-00-testing.md

+## The `assert!` macro
+
+So why does our do-nothing test pass? Any test which doesn't `panic!` passes,
+and any test that does `panic!` fails. Let's make our test fail:


Should we remind them where we originally talked about panic!?

I'm fine with it, but am not sure we need to mention this stuff every time. @carols10cents ?

I feel like with the amount we talk about panic in the error handling chapter, we shouldn't have to mention it again here. It'll be in the table of contents too.

sophiajt · 2016-11-11T20:43:44Z

src/chXX-01-unit-testing.md

+have to follow the usual visibility rules. Because we're in an inner module,
+we need to bring our test function into scope. This can be annoying if you have
+a large module, and so this is a common use of globs. Let's change our
+`src/lib.rs` to make use of it:


Do we talk about needing to use use like this in a different chapter, like the one talking about modules? It seems that while this is review it's almost like we're talking about it for the first time.

Hm, do you have any ideas on how to make it feel less first-timey?

sophiajt · 2016-11-11T20:50:35Z

Yeah, seems like the ordering bit, and the minor points of confusions, are the remaining tasks. As a whole this feels like it's already pretty far along and just needs the polish to wrap it up.

steveklabnik · 2016-11-14T15:57:55Z

Got a PR with some stuff that i want to make sure is included here too rust-lang/rust#37766 (review)

steveklabnik · 2016-11-14T19:44:07Z

Fixed some nits and added a tiny bit at the end, @carols10cents , take it away 😄

in my court now

that goes in the 'creating a library' section

…oncerns

Reorganize a bit, make some of the examples a bit more concrete

carols10cents · 2016-11-25T22:52:10Z

src/ch11-03-test-organization.md

+test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured
+```
+
+#### Submodules in integration tests


I'm still not sure about this section. My force push ate my comments tho, luckily I had them in another window:

I'm still confused about integration test files and crates and the relationship thereof :(

The beginning of the integration test section says:

If you make [a tests directory], and put .rs files inside, Cargo will compile each of [the .rs files] as an individual crate.

and:

We now have an extern crate adder at the top. This is because each test in the tests directory is an entirely separate crate, and so we need to import our library.

I read this as "each file in tests is compiled into a crate that is independent of each of the other files in tests.

However, in this section, we're demonstrating that if you extract a module into tests/common.rs, because that's a .rs file in the tests directory too, common will be compiled into a crate and get its own section in the tests output.

But we were able to use common::helper() in tests/integration_test.rs without having to say extern crate helper, so does that mean the separate crates in tests are compiled in... like... the same context? Like they all have each other imported as extern crate would already? I'm going to research this too but wanted to confirm with you....
@carols10cents
carols10cents 23 minutes ago collaborator

Ok jake has helped me to understand that it's compiling tests/common.rs as a separate crate AND as a module of tests/integration_test.rs because of the mod common..... I'm still struggling with this because we say it's a problem but we don't demonstrate any problems.
@carols10cents
carols10cents 17 minutes ago collaborator

I found one example of someone doing this in IRC, but the problem is slightly different than the problem presented here: https://botbot.me/mozilla/rust/2016-03-02/?msg=61394193&page=20

They want to make one tests/lib.rs and then include other test files as mods in there, as well as have helper mods declared in tests/lib.rs and usable in the other test file mods, rather than what's presented here, which is having multiple top-level crates that could all havemod common and work just fine....

And add a section about integration testing binary crates

carols10cents · 2016-11-26T19:06:38Z

Ok, after thinking about this some more, I think the content about modules in the tests directory is valuable, but the example is muddying the point more than it's helping. So I've shortened that section.

In my research trying to find logs of people on IRC (in #rust only since #rust-beginners isn't logged, and I totally understand why it isn't) being in this situation and what they were doing, I actually found quite a few more people asking how to integration test binary crates, which isn't possible, so I decided to add a bit about what to do in that situation too.

So I think I'm done messing around with this and would love thoughts ❤️

daschl · 2016-11-27T09:40:54Z

src/ch11-01-writing-tests.md

+
+```rust,ignore
+// assert_eq
+if left_val == right_val {


I think assert_eq and assert_ne are mixed up here since they panic on the inverted == vs !=

Oooooooh great catch!!! 👀 You're totally right!!!

daschl · 2016-11-27T09:43:42Z

great work! One question for me came up when reading the "writing tests" chapter is that maybe a paragraph should clarify how that relates to the Eq trait and so forth - maybe show how to assert two custom structs?

daschl · 2016-11-27T10:10:03Z

src/ch11-02-running-tests.md

+
+test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured
+```
+


Would it be also possible to expand this a little bit to explain how to run certain module tests? Since very often you want to run all the tests of a specific module/namespace.

Would it be also possible to expand this a little bit to explain how to run certain module tests? Since very often you want to run all the tests of a specific module/namespace.

That's a great idea!! Will do!!

@daschl

Thank you @daschl!!!

carols10cents · 2016-11-27T15:46:36Z

One question for me came up when reading the "writing tests" chapter is that maybe a paragraph should clarify how that relates to the Eq trait and so forth - maybe show how to assert two custom structs?

Ooooh great point!!! I will work this in :) Thank you for reviewing this!!! ❤️

Also make the pseudocode a bit more accurate

mgattozzi · 2016-11-27T16:47:35Z

src/ch11-00-testing.md

+borrow checking that we've seen it's capable of doing. What Rust *can't* check
+for us is that we've implemented this function to return the argument plus two
+and not the argument plus 10 or the argument minus 50! That's where tests come
+in: we can write tests that, for example, pass `3` to the `add_two` function


You might just want to write this as 'where tests come in. We can write tests that'
Having the : made the flow a bit weird. That or 'come in: we can write, for example' using tests as a word twice so close to each other trips the flow up.

¯\_(ツ)_/¯ none of these versions bother me, so might as well change it :)

mgattozzi · 2016-11-27T16:48:47Z

src/ch11-00-testing.md

+to our code to make sure we didn't change any existing behavior from what the
+tests specify it should be.
+
+Testing is a skill, and we cannot hope to cover everything about how to write


The comma after skill on this line is not needed. If it was a list then having the oxford comma would be good but this is just a conjunction of two sentences.

Conjunctions of two sentences get commas, though. https://owl.english.purdue.edu/owl/resource/607/02

steveklabnik

Looks good! One nit and one thing for the future...

steveklabnik · 2016-11-28T16:24:37Z

src/ch11-01-writing-tests.md

+didn't fail for a different reason than the one you were expecting. To help
+with this, an optional `expected` parameter can be added to the `should_panic`
+attribute. The test harness will make sure that the failure message contains
+the provided text. A safer version of Listing 11-1 would be the following, in


"safe" is probably a bad word here, given rust's focus on a different kind of safety. What about "more robust"?

steveklabnik · 2016-11-28T16:27:38Z

src/ch11-01-writing-tests.md

+
+```rust
+#[test]
+#[should_panic(expected = "do not lie on character boundary")]


rust-lang/rust#38052

(we should leave this for now, but it'll need fixed at some point)

steveklabnik · 2016-11-28T16:33:42Z

carols10cents self-assigned this Oct 7, 2016

carols10cents previously requested changes Oct 10, 2016

View reviewed changes

carols10cents removed their assignment Oct 10, 2016

steveklabnik mentioned this pull request Oct 23, 2016

Update testing.md to reflect changes to cargo new rust-lang/rust#37368

Merged

steveklabnik force-pushed the testing branch from c0b82b5 to bbdbebe Compare November 8, 2016 21:16

trotter added a commit to trotter/rust that referenced this pull request Nov 10, 2016

Remove mod tests from earlier sections

35903bb

The narrative flows better if we follow what @steveklabnik is doing in rust-lang/book#288. Therefore, I completely copied it.

sophiajt reviewed Nov 11, 2016

View reviewed changes

carols10cents self-assigned this Nov 14, 2016

steveklabnik and others added 11 commits November 25, 2016 15:24

set up

0097f8c

initial import/re-write of testing chapter

a85af06

rename to unit testing

62bfe3a

add error message for assert_eq

5428b7f

Remove 'documentation tests' bit

aefa195

that goes in the 'creating a library' section

add stuff about running subsets of tests

a49d644

remove myself

2f4c61d

swap order of sections to resolve @carols10cents @jonathandturner's c…

71b87e0

…oncerns

Add notes about parallelism

182fa3d

Remove the Debugging chapter that snuck back in

97ffe9f

Carol's edits to the testing chapter

d1107ec

Reorganize a bit, make some of the examples a bit more concrete

carols10cents force-pushed the testing branch from bb20eda to d1107ec Compare November 25, 2016 22:49

carols10cents reviewed Nov 25, 2016

View reviewed changes

Shorten section about integration test modules

a68f2fa

And add a section about integration testing binary crates

carols10cents assigned steveklabnik and unassigned carols10cents Nov 26, 2016

daschl reviewed Nov 27, 2016

View reviewed changes

Fix error about what the eq and ne macros actually do

5e49128

Thank you @daschl!!!

carols10cents added 3 commits November 27, 2016 11:02

Add a paragraph about needing PartialEq and Debug

316db3e

Also make the pseudocode a bit more accurate

Show test name matching works for module names too

e3554b7

Adding some listing numbers

d242dc4

mgattozzi reviewed Nov 27, 2016

View reviewed changes

Change colon to period

b45eae6

steveklabnik commented Nov 28, 2016

View reviewed changes

More robust, not safer

093935f

carols10cents merged commit 9b14e37 into master Nov 28, 2016

carols10cents deleted the testing branch November 28, 2016 16:51


		## Testing equality

		Rust provides a pair of macros, `assert_eq!` and `assert_ne!`, that compares


		test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured
		```

new chapter: testing #288

new chapter: testing #288

Conversation

steveklabnik commented Oct 7, 2016

steveklabnik commented Oct 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carols10cents Nov 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carols10cents commented Oct 10, 2016

steveklabnik commented Oct 10, 2016

steveklabnik commented Nov 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sophiajt commented Nov 11, 2016

steveklabnik commented Nov 14, 2016

steveklabnik commented Nov 14, 2016

Choose a reason for hiding this comment

carols10cents commented Nov 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daschl commented Nov 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carols10cents commented Nov 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

steveklabnik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

steveklabnik commented Nov 28, 2016

carols10cents Nov 14, 2016 •

edited

Loading