Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address testthat deprecation warnings + drop mockery #589

Merged
merged 7 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ Suggests:
xml2 (>= 1.0.0),
parallel,
memoise,
mockery,
covr,
box (>= 1.2.0)
License: MIT + file LICENSE
Expand Down
3 changes: 3 additions & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ export(to_sonarqube)
export(value)
export(zero_coverage)
import(methods)
importFrom(httr,RETRY)
importFrom(httr,content)
importFrom(httr,upload_file)
Comment on lines +35 to +37
Copy link
Contributor Author

Choose a reason for hiding this comment

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

importFrom(stats,aggregate)
importFrom(stats,na.omit)
importFrom(stats,na.pass)
Expand Down
12 changes: 6 additions & 6 deletions R/codecov.R
Original file line number Diff line number Diff line change
Expand Up @@ -228,12 +228,12 @@ codecov <- function(...,

coverage_json <- to_codecov(coverage)

httr::content(httr::RETRY("POST",
url = codecov_url,
query = codecov_query,
body = coverage_json,
encode = "json",
httr::config(http_version = curl_http_1_1())))
content(RETRY("POST",
url = codecov_url,
query = codecov_query,
body = coverage_json,
encode = "json",
httr::config(http_version = curl_http_1_1())))
}

curl_http_1_1 <- function() {
Expand Down
21 changes: 18 additions & 3 deletions R/compiled.R
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
# this does not handle LCOV_EXCL_START ect.
parse_gcov <- function(file, package_path = "") {
if (!file.exists(file)) {
if (!source_file_exists(file)) {
return(NULL)
}

lines <- readLines(file)
lines <- read_lines_impl(file)
source_file <- rex::re_matches(lines[1], rex::rex("Source:", capture(name = "source", anything)))$source

# retrieve full path to the source files
source_file <- normalize_path(source_file)

# If the source file does not start with the package path or does not exist ignore it.
if (!file.exists(source_file) || !grepl(rex::rex(start, rex::regex(paste0(rex::escape(package_path), collapse = "|"))), source_file)) {
if (!source_file_exists(source_file) || !grepl(rex::rex(start, rex::regex(paste0(rex::escape(package_path), collapse = "|"))), source_file)) {
return(NULL)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These changes are required to remove the mockery dependency. I kept the commit self-contained.

But basically, with mockery::stub(), we can mock a function contained to a function. For example, mocking file.exists() only inside parse_gcov(). However, testthat::local_mocked_bindings() doesn't provide this ability.

The workaround I found is to create a wrapper function for file.exists() (in this case source_file_exists().) that I will only use inside parse_gcov(). This way, I can mock source_file_exists() to only mock file.exists() inside parse_gcov().

Copy link
Member

Choose a reason for hiding this comment

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

So for base functions you can assign a NULL binding in the package rather than using a wrapper function. See https://testthat.r-lib.org/reference/local_mocked_bindings.html?q=local_mock#base-functions. This works because when the name is used as a function call the NULL assigned value is not a callable function, so it is not included in the lookup and the base version gets used instead. Then when the mocking happens it can assign a function to the variable in the package's namespace.

}

Expand Down Expand Up @@ -46,9 +46,24 @@ parse_gcov <- function(file, package_path = "") {
# There are no functions for gcov, so we set everything to NA
functions <- rep(NA_character_, length(values))

line_coverages_impl(source_file, matches, values, functions)
}

# For mocking (only use it within parse_gcov())
line_coverages_impl <- function(source_file, matches, values, functions) {
line_coverages(source_file, matches, values, functions)
}

# For mocking (only use it within parse_gcov())
source_file_exists <- function(path) {
file.exists(path)
}

# For mocking (only use it within parse_gcov())
read_lines_impl <- function(path) {
readLines(path)
}

clean_gcov <- function(path) {
src_dir <- file.path(path, "src")

Expand Down
6 changes: 3 additions & 3 deletions R/coveralls.R
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ coveralls <- function(..., coverage = NULL,
coverage_json <- to_coveralls(coverage,
repo_token = repo_token, service_name = service)

result <- httr::RETRY("POST", url = coveralls_url,
body = list(json_file = httr::upload_file(to_file(coverage_json))))
result <- RETRY("POST", url = coveralls_url,
body = list(json_file = upload_file(to_file(coverage_json))))

content <- httr::content(result)
content <- content(result)
if (isTRUE(content$error)) {
stop("Failed to upload coverage data. Reply by Coveralls: ", content$message)
}
Expand Down
4 changes: 4 additions & 0 deletions R/covr.R
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@
#' @importFrom utils capture.output getSrcFilename relist str head
NULL

# For mocking
#' @importFrom httr content RETRY upload_file
NULL

the <- new.env(parent = emptyenv())

the$replacements <- list()
Expand Down
7 changes: 6 additions & 1 deletion R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -322,11 +322,16 @@ env_path <- function(...) {
}

normalize_path <- function(x) {
path <- normalizePath(x, winslash = "/", mustWork = FALSE)
path <- normalize_path_impl(x)
# Strip any trailing slashes as they are invalid on windows
sub("/*$", "", path)
}

# For mocking (inside normalize_path)
normalize_path_impl <- function(path) {
normalizePath(path, winslash = "/", mustWork = FALSE)
}

temp_dir <- function() {
normalize_path(tempdir())
}
Expand Down
Loading
Loading