Skip to content

Conversation

hadley
Copy link
Member

@hadley hadley commented Sep 8, 2025

No description provided.

Copy link

claude bot commented Sep 8, 2025

Claude finished @hadley's task —— View job


Code Review Complete

Reviewing PR changes for potential bugs or issues:

  • Analyze DESCRIPTION changes
  • Review R code changes (mock-oo.R, mock2.R)
  • Check documentation updates
  • Review vignette changes
  • Provide feedback

Issues found:

vignettes/challenging-tests.Rmd:147 - Incomplete sentence "And" followed by code block. The text should complete the thought or be removed.

LGTM for all other changes - documentation improvements, vignette references to vignette("mocking"), and RoxygenNote version bump are appropriate.

Copy link
Contributor

@maelle maelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

})
```

If you don't care about reproducing the output of `continue()` and just want to recreate its return value, you can use `mock_output_sequence()`. This creates a function that each time it's called returns the next input supplied to `mock_output_sequence()`. The following code shows how it works and how you might use it to test `readline()`:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the second sentence unclear.

Suggested change
If you don't care about reproducing the output of `continue()` and just want to recreate its return value, you can use `mock_output_sequence()`. This creates a function that each time it's called returns the next input supplied to `mock_output_sequence()`. The following code shows how it works and how you might use it to test `readline()`:
If you don't care about reproducing the output of `continue()` and just want to recreate its return value, you can use `mock_output_sequence()`. This creates a function that returns the input supplied to `mock_output_sequence()` in sequence: the first input at the first call, the second input at the second call, etc. The following code shows how it works and how you might use it to test `readline()`:


## Do you need it?

But before you read the rest of the vignette and dive into the full details of creating a fully 100% correct expectation, consider if you can get away with a simpler wrapper. For example, take this expectation from tidytext:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not start with an example of a wrapper that is fine? For instance it could be

expect_df <- function(tbl) {
  expect_s3_class(tbl, "data.frame")
}

Or here is a real example: https://github.com/ropensci/aorsf/blob/2b7fa72bff5cd4e8b2984358e35d0938e50540b0/tests/testthat/helper-orsf.R#L259


## Do you need it?

But before you read the rest of the vignette and dive into the full details of creating a fully 100% correct expectation, consider if you can get away with a simpler wrapper. For example, take this expectation from tidytext:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
But before you read the rest of the vignette and dive into the full details of creating a fully 100% correct expectation, consider if you can get away with a simpler wrapper. For example, take this expectation from tidytext:
But before you read the rest of the vignette and dive into the full details of creating a 100% correct expectation, consider if you can get away with a simpler wrapper. For example, take this expectation from tidytext:

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 this pull request may close these issues.

3 participants