Make require_testthat helper function #2587
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AshesITR
reviewed
May 28, 2024
R/expect_lint.R
Outdated
| require_testthat <- function(name) { | ||
| if (!requireNamespace("testthat", quietly = TRUE)) { | ||
| stop( # nocov start | ||
| name, "is designed to work within the 'testthat' testing framework, but 'testthat' is not installed.", |
There was a problem hiding this comment.
Doesn't stop() use sep = "", butchering the error message like this?
There was a problem hiding this comment.
You are correct. Added a space.
NEWS.md
Outdated
| @@ -56,6 +56,7 @@ | |||
| * `make_linter_from_xpath()` errors up front when `lint_message` is missing (instead of delaying this error until the linter is used, #2541, @MichaelChirico). | |||
| * `paste_linter()` is extended to recommend using `paste()` instead of `paste0()` for simply aggregating a character vector with `collapse=`, i.e., when `sep=` is irrelevant (#1108, @MichaelChirico). | |||
| * `expect_no_lint()` was added as new function to cover the typical use case of expecting no lint message, akin to the recent {testthat} functions like `expect_no_warning()` (#2580, @F-Noelle). | |||
| * `expect_lint_free()` and other functions that rely on the {testthat} framework now have a consistent error message. (#2585, @F-Noelle). | |||
There was a problem hiding this comment.
I would put this under a ## Notes section (see other releases)
There was a problem hiding this comment.
Shifted the item to notes.
R/expect_lint.R
Outdated
| @@ -158,3 +155,13 @@ expect_lint_free <- function(...) { | |||
|
|
|||
| invisible(result) | |||
| } | |||
|
|
|||
| # Helper function to check if testthat is installed. | |||
| require_testthat <- function(name) { | |||
There was a problem hiding this comment.
I would not require the argument, e.g. name = sys.call(sys.parent()), or possibly re-using/unifying with linter_auto_name(). Then just require_testthat().
There was a problem hiding this comment.
I tested this a bit. linter_auto_name(-5L) seems to do the trick.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses issues (#2585)