CH11-01 - Using Result<T, E> in Tests - Recommendation of question mark is confusing

[djacu]

• I have searched open and closed issues and pull requests for duplicates, using these search terms:
  • Using Result Ch11
  • Using Result
  • question mark operator
• I have checked the latest main branch to see if this has already been fixed, in this file:
  • Yes

URL to the section(s) of the book with this problem:
https://doc.rust-lang.org/book/ch11-01-writing-tests.html#using-resultt-e-in-tests

Description of the problem:
In the following paragraphs, the first paragraph indicates that using the question mark operator is useful for tests that fail but the second paragraph advises against it. There are no examples given so it is difficult as a new reader to determine if one of the statements is outdated and false, or if both are true and there are idiomatic ways to use the question mark operator.

Writing tests so they return a Result<T, E> enables you to use the question mark operator in the body of tests, which can be a convenient way to write tests that should fail if any operation within them returns an Err variant.

You can’t use the #[should_panic] annotation on tests that use Result<T, E>. To assert that an operation returns an Err variant, don’t use the question mark operator on the Result<T, E> value. Instead, use assert!(value.is_err()).

Suggested fix:
If both statements are true, a simple example would be helpful. E.g. use the question mark operator like this but not like this.

[chriskrycho]

Thanks for filing the issue, and sorry you found that confusing!

The text here says that you can use the ? operator if you want the test to fail if there is an Err, but that you need to use assert! if you want the test to pass when there is an Err. I would be happy to discuss here whether there is a tweak to the phrasing which makes that clearer, but there is definitely no contradiction between the two paragraphs even as they are now!

[djacu]

Not a problem at all! If the first time I am confused while reading this book is in the 11th chapter, I think that is a testament to how good the book is. This book has been a joy to read. :)

Ahh I believe I understand now. After reading your explanation and more carefully reading the book, it make sense to me.

I think adding an additional phrase the explicitly calls out the desired outcome would add some clarity. E.g. "If we expect a test to pass..." and "If we expect a test to fail". What do you think about the following?

Writing tests so they return a Result<T, E> enables you to use the question mark operator in the body of tests, if you expect the test to pass. This can be a convenient way to write tests that should fail if any operation within them returns an Err variant.

If you expect a test to fail, you can’t use the #[should_panic] annotation on tests that use Result<T, E>. To assert that an operation returns an Err variant, don’t use the question mark operator on the Result<T, E> value. Instead, use assert!(value.is_err()).

[chriskrycho]

Explicitly calling out the desired outcome makes sense. I think the specific phrasing around expecting the test to pass or fail is also not quite right, though: after all, in the cases we’re looking at here, the test should pass in both cases! Consider a (silly!) case like this:

// empty struct just for the demo
#[derive(Debug)]
struct SomeError;

fn demo(pass: bool) -> Result<(), SomeError> {
    if pass {
        Ok(())
    } else {
        Err(SomeError)
    }
}

/// This test shouuld pass, of course.
#[test]
fn test_demo_ok_passes() -> Result<(), SomeError> {
    demo(true)
}

/// This test should *also* pass, though! This is the “desired behavior” of the
/// function: to return an `Err` when passed `false`.
#[test]
fn test_demo_err_passes() {
    assert!(demo(false).is_err());
}

So the issue is not whether the test passes or fails, but what it is testing for: Ok or Err. I will think on the phrasing here to see how we can make it more explicit!

[djacu]

You're totally correct. I'll ponder it as well.

[chriskrycho]

I let this one simmer for a while, as promised, and re-read the section here again, and I ultimately think the phrasing is fine as is. Thanks for flagging up your confusion here, though—that’s how we improve this.