Improve custom expectation docs #2143
Conversation
|
Could you review this one too please @EmilHvitfeldt? |
vignettes/custom-expectation.Rmd
Outdated
| ### Capture value and label | ||
| The first step in any expectation is to use `quasi_label()` to capture a "labelled value", i.e. an list that contains both the value (`$val`) for testing and a label (`$lab`) for messaging. This is a pattern that exists for fairly esoteric reasons; you don't need to understand, just copy and paste it 🙂. | ||
|
|
||
| Next you need to check each way that `object` could be broken. In most cases, it's easier to check for problems one by one, using early returns to `fail()` when any expectation is violated as that makes it easier to write failure messages. It's good practice to state both what the object is and what you expected in your failures. |
There was a problem hiding this comment.
Next you need to check each way that
objectcould be broken.
I think "broken" sends the wrong message. Maybe something like
Next you need to check each way that the
objectcould violate the stated expectation you are testing for.
There was a problem hiding this comment.
I liked violated expectation as a term. Thanks!
vignettes/custom-expectation.Rmd
Outdated
| act <- quasi_label(rlang::enquo(object)) | ||
| if (!is.atomic(act$val) || !is.list(act$val)) { | ||
| if (!is.atomic(act$val) && !is.list(act$val)) { |
There was a problem hiding this comment.
| if (!is.atomic(act$val) && !is.list(act$val)) { | |
| if (!(is.atomic(act$val) || is.list(act$val))) { |
weakly held opinion. since it isn't computational crucial, i think this is more readable
There was a problem hiding this comment.
I find that hard to read too because of all the parens. But for this case I can just use rlang::is_vector() instead. (And that's generally what I'd recommend - if you don't like the and-not form, write an informatively named helper function or intermediate variable.)
| act_n <- length(act$val) | ||
| if (act_n != n) { | ||
| msg <- sprintf("%s has length %i, not length %i.", act$lab, act_n, n) | ||
| return(fail(msg, trace_env = trace_env)) | ||
| } | ||
| expect_snapshot_failure(expect_length(x, 11)) | ||
| }) | ||
| pass(act$val) |
There was a problem hiding this comment.
maybe a note should be added that both fail() and pass() should be used in the helper, as it will be used in return() explicitly or otherwise.
There was a problem hiding this comment.
Good point; tried to clarify below
Co-authored-by: Emil Hvitfeldt <[email protected]>
Fixes #2113. Fixes #2132. Fixes #2072.