✨ More proofreading ✨

Author: hadley

vignettes/challenging-tests.Rmd

@@ -20,7 +20,7 @@ snapper$start_file("snapshotting.Rmd", "test")
 Sys.setenv(TESTTHAT_PKG = "testthat")
 ```
 
-This vignette is a quick reference guide for testing challenging functions. It's organised by the problem, rather than technique used to solve it, so you can quickly skim the whole vignette, spot the problem you're facing, then learn more about useful tools for solving it.
+This vignette is a quick reference guide for testing challenging functions. It's organised by the problem, rather the than technique used to solve it, so you can quickly skim the whole vignette, spot the problem you're facing, then learn more about useful tools for solving it.
 
 ## Options and environment variables
 
@@ -67,7 +67,8 @@ You can skip a test without passing or failing if it's not possible to run it in
 
 ## HTTP requests
 
-If you're trying to test functions that rely on HTTP requests, we recommend using {vcr} or {httptest2}. These packages provide the ability to record and then later reply HTTP requests so that you can test without an active internet connection. If your package is going to CRAN, we highly recommend either using one of these packages or using `skip_on_cran()` for your internet-facing tests. This ensures that your package won't break on CRAN just because the service you're using is temporarily down.
+If you're trying to test functions that rely on HTTP requests, we recommend using {vcr} or {httptest2}.
+These packages provide the ability to record and then later replay HTTP requests so that you can test without an active internet connection. If your package is going to CRAN, we highly recommend either using one of these packages or using `skip_on_cran()` for your internet-facing tests. This ensures that your package won't break on CRAN just because the service you're using is temporarily down.
 
 ## User interaction
 
@@ -144,4 +145,4 @@ Learn more in `vignette("mocking")`.
 
 ## User-facing text
 
-Errors, warnings, and other user-facing text should be tested to ensure they're consistent and actionable. Obviously you can't test this 100% automatically, but you can ensure that such messaging is clearly shown in PRs so another human can take a look. This is point of snapshot tests; learn more in `vignette("snapshotting")`.
+Errors, warnings, and other user-facing text should be tested to ensure they're consistent and actionable. Obviously you can't test this 100% automatically, but you can ensure that such messaging is clearly shown in PRs so another human can take a look. This is the point of snapshot tests; learn more in `vignette("snapshotting")`.

vignettes/custom-expectation.Rmd

@@ -193,7 +193,7 @@ expect_s3_class(x1, 1)
 
 ## Repeated code
 
-As you write more expectations, you might discover repeated code that you want to extract out into a helper. Unfortunately, creating helper functions is not straightforward in testthat because every `fail()` captures the calling environment in order to give maximally useful tracebacks. Because getting this right is not critical (you'll just get a slightly suboptimal traceback in the case of failure), we don't recommend bothering. However, we document it here because it's important to get it right in testthat itself.
+As you write more expectations, you might discover repeated code that you want to extract out into a helper. Unfortunately, creating helper functions is not straightforward in testthat because every `fail()` captures the calling environment in order to give maximally useful tracebacks. Because getting this right is not critical (you'll just get a slightly suboptimal traceback in the case of failure), we don't recommend bothering in most cases. However, we document it here because it's important to get it right in testthat itself.
 
 The key challenge is that `fail()` captures a `trace_env` which should be the execution environment of the expectation. This usually works, because the default value of `trace_env` is `caller_env()`. But when you introduce a helper, you'll need to explicitly pass it along:
 

vignettes/mocking.Rmd

@@ -51,7 +51,7 @@ check_installed <- function(pkg, min_version = NULL) {
 }
 ```
 
-Now that we've written this function, we want to test it. There a lot of ways we might tackle this, but I think it's reasonable to start by testing the case without `min_version`. To do this we need to come up with a package we know is installed, and a package we know isn't installed:
+Now that we've written this function, we want to test it. There are a lot of ways we might tackle this, but I think it's reasonable to start by testing the case without `min_version`. To do this we need to come up with a package we know is installed, and a package we know isn't installed:
 
 ```{r}
 test_that("check_installed() checks package is installed", {
@@ -84,7 +84,7 @@ test_that("check_installed() checks minimum version", {
 })
 ```
 
-But it's starting to feel like we've accumulated a bunch of potentially fragile hacks. So let's see how we could could make these tests more robust with mocking. First we need to add `requireNamspace` and `packageVersion` bindings in our package. This is needed because `requireNamespace` and `packageVersion` are base functions: 
+But it's starting to feel like we've accumulated a bunch of potentially fragile hacks. So let's see how we could make these tests more robust with mocking. First we need to add `requireNamespace` and `packageVersion` bindings in our package. This is needed because `requireNamespace` and `packageVersion` are base functions: 
 ```{r}
 requireNamespace <- NULL
 packageVersion <- NULL
@@ -102,7 +102,7 @@ test_that("check_installed() checks package is installed", {
 })
 ```
 
-For the second test, we mock `requireNamepace()` to return `TRUE`, and then `packageVersion()` to return a fixed version number. Together this simulates that version 2.0.0 of any package is installed and makes our snapshot rely only on the state set within the test.
+For the second test, we mock `requireNamespace()` to return `TRUE`, and then `packageVersion()` to return a fixed version number. Together this simulates that version 2.0.0 of any package is installed and makes our snapshot rely only on the state set within the test.
 
 ```{r}
 test_that("check_installed() checks minimum version", {
@@ -158,7 +158,7 @@ Here we pretend that there are no reverse dependencies (revdeps) for the package
 
 ### Managing time
 
-`httr2::req_throttle()` prevents multiple requests from being made too quickly, using a tool called a leaky token bucket. This tool is inextricably tied to real time because you want to allow more requests as time elapses. So how do you test this? I started by using `Sys.sleep()` but this either made my tests both slow (because I'd sleep for a second or two) and unreliable (because sometime more time elapsed than I expected). Eventually I figured out that I could "manually control" time by using a [mocked function](https://github.com/r-lib/httr2/blob/main/tests/testthat/test-req-throttle.R) that returns the value of a variable I control. This allows me to manually advance time and carefully test the implications.
+but this either made my tests both slow (because I'd sleep for a second or two) and unreliable (because sometimes more time elapsed than I expected). Eventually I figured out that I could "manually control" time by using a [mocked function](https://github.com/r-lib/httr2/blob/main/tests/testthat/test-req-throttle.R) that returns the value of a variable I control. This allows me to manually advance time and carefully test the implications.
 
 You can see the basic idea with a simpler example. Let's first begin with a function that returns the "unix time", the number of seconds elapsed since midnight on Jan 1 1970. This is easy to compute, but will make some computations simpler later as well as providing a convenient function to mock.
 

vignettes/skipping.Rmd

@@ -76,7 +76,7 @@ skip_if_dangerous <- function() {
 
 ## Embedding `skip()` in package functions
 
-Another useful technique that can sometimes be useful is to build a `skip()` directly into a package function.
+Another potentially useful technique is to build a `skip()` directly into a package function.
 For example take a look at [`pkgdown:::convert_markdown_to_html()`](https://github.com/r-lib/pkgdown/blob/v2.0.7/R/markdown.R#L95-L106), which absolutely, positively cannot work if the Pandoc tool is unavailable:
 
 ```{r eval = FALSE}

vignettes/snapshotting.Rmd

@@ -42,7 +42,7 @@ snapper$start_file("snapshotting.Rmd", "test")
 
 ## Basic workflow
 
-We'll illustrate the basic workflow with a simple function that generates an HTML heading.
+We'll illustrate the basic workflow with a simple function that generates an HTML bullets.
 It can optionally include an `id` attribute, which allows you to construct a link directly to that heading.
 
 ```{r}
@@ -237,7 +237,8 @@ test_that("actionable feedback if some or all arguments named", {
 
 Sometimes part of the output varies in ways that you can't easily control. In many cases, it's convenient to use mocking (`vignette("mocking")`) to ensure that every run of the function always produces the same output. In other cases, it's easier to manipulate the text output with a regular expression or similar. That's the job of the `transform` argument which should be passed a function that takes a character vector of lines, and returns a modified vector.
 
-This type of problem often crops up when you are testing a function that gives feedback about a path. In your tests, you'll typically use a temporary path (e.g. from `withr::local_tempfile()`) so if you display the path in a snapshot, it will be different every time. For example, consider this "safe" version of `writeLines()` that requires to explicitly opt-in to overwriting an existing file:
+This type of problem often crops up when you are testing a function that gives feedback about a path. In your tests, you'll typically use a temporary path (e.g. from `withr::local_tempfile()`) so if you display the path in a snapshot, it will be different every time.
+For example, consider this "safe" version of `writeLines()` that requires you to explicitly opt-in to overwriting an existing file:
 
 ```{r}
 safe_write_lines <- function(lines, path, overwrite = FALSE) {
@@ -314,7 +315,7 @@ These are sound defaults that we have found useful to minimise spurious differen
 
 ### Snapshotting graphics
 
-If you need to test graphical output, {vdiffr}. vdiffr is used to test ggplot2, and incorporates everything we know about high-quality graphics tests that minimise false positives.
+If you need to test graphical output, use {vdiffr}. vdiffr is used to test ggplot2, and incorporates everything we know about high-quality graphics tests that minimise false positives. Graphics testing is still often fragile, but using vdiffr means you will avoid all the problems we know how to avoid.
 
 ### Snapshotting values
 
@@ -384,7 +385,7 @@ This section describes some of the previous attempts and why we believe the new
 
     -   It's relatively coarse grained, which means tests that use it tend to keep growing and growing.
 
--   `expect_known_output()` is finer grained version of `verify_output()` that captures output from a single function.
+-   `expect_known_output()` is a finer grained version of `verify_output()` that captures output from a single function.
     The requirement to produce a path for each individual expectation makes it even more painful to use.
 
 -   `expect_known_value()` and `expect_known_hash()` have all the disadvantages of `expect_known_output()`, but also produce binary output meaning that you can't easily review test differences in pull requests.

vignettes/test-fixtures.Rmd

@@ -71,7 +71,7 @@ test_that("print() respects digits option", {
 })
 ```
 
-If you write a lot of code like this in your tests, you might decide you want a helper function or **test fixture** that reduces the duplication. Fortunately withr's local functions us to solve this problem by providing an `.local_envir` or `envir` argument that controls when cleanup occurs. The exact details of how this works are rather complicated, but fortunately there's a common pattern you can use without understanding all the details. Your helper function should always have an `env` argument that defaults to `parent.frame()`, which you pass to the `.local_envir` argument of `local_()`:
+If you write a lot of code like this in your tests, you might decide you want a helper function or **test fixture** that reduces the duplication. Fortunately withr's local functions allow us to solve this problem by providing an `.local_envir` or `envir` argument that controls when cleanup occurs. The exact details of how this works are rather complicated, but fortunately there's a common pattern you can use without understanding all the details. Your helper function should always have an `env` argument that defaults to `parent.frame()`, which you pass to the `.local_envir` argument of `local_()`:
 
 ```{r}
 local_digits <- function(sig_digits, env = parent.frame()) {
@@ -113,7 +113,7 @@ Notice how `pi` prints differently before and after the call to `sloppy()`. Call
 
 ### `on.exit()`
 
-The first function you need to know about is base R's `on.exit()`. `on.exit()` calls the code to supplied to its first argument when the current function exits, regardless of whether it returns a value or errors. You can use `on.exit()` to clean up after yourself by ensuring that every mess-making function call is paired with an `on.exit()` call that cleans up.
+The first function you need to know about is base R's `on.exit()`. `on.exit()` calls the code supplied to its first argument when the current function exits, regardless of whether it returns a value or errors. You can use `on.exit()` to clean up after yourself by ensuring that every mess-making function call is paired with an `on.exit()` call that cleans up.
 
 We can use this idea to turn `sloppy()` into `neat()`:
 
@@ -145,7 +145,7 @@ pi
 
 There are three main drawbacks to `on.exit()`:
 
--   You should always call it with `add = TRUE` and `after = FALSE`. These ensure that the call is **added** to the list of deferred tasks (instead of replaces) and is added to the **front** of the stack (not the back, so that cleanup occurs in reverse order to setup). These arguments only matter if you're using multiple `on.exit()` calls, but it's a good habit to always use them to avoid potential problems down the road.
+-   You should always call it with `add = TRUE` and `after = FALSE`. These ensure that the call is **added** to the list of deferred tasks (instead of replacing them) and is added to the **front** of the stack (not the back, so that cleanup occurs in reverse order to setup). These arguments only matter if you're using multiple `on.exit()` calls, but it's a good habit to always use them to avoid potential problems down the road.
 
 -   It doesn't work outside a function or test. If you run the following code in the global environment, you won't get an error, but the cleanup code will never be run:
 
@@ -293,7 +293,7 @@ test_that("message2() output depends on verbose option", {
 
 One place that we use test fixtures extensively is in the usethis package ([usethis.r-lib.org](https://usethis.r-lib.org)), which provides functions for looking after the files and folders in R projects, especially packages. Many of these functions only make sense in the context of a package, which means to test them, we also have to be working inside an R package. We need a way to quickly spin up a minimal package in a temporary directory, then test some functions against it, then destroy it.
 
-To solve this problem we create a test fixture, which we place in `R/test-helpers.R` so that's it's available for both testing and interactive experimentation:
+To solve this problem we create a test fixture, which we place in `R/test-helpers.R` so that it's available for both testing and interactive experimentation:
 
 ```{r, eval = FALSE}
 local_create_package <- function(dir = file_temp(), env = parent.frame()) {