Skip to content

Commit 4ed4d8e

Browse files
authored
Improve test coverage (#87)
* available java versions mock for mocked tests * fix java download and move helper outside the function * java download mocked tests * java install mocked tests
1 parent 55dc10a commit 4ed4d8e

File tree

5 files changed

+398
-27
lines changed

5 files changed

+398
-27
lines changed

R/java_download.R

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -31,31 +31,6 @@ java_download <- function(
3131
force = FALSE,
3232
temp_dir = FALSE
3333
) {
34-
# Download distribution and check MD5 checksum
35-
download_dist_check_md5 <- function(url, dest_file, quiet) {
36-
curl::curl_download(url, dest_file, quiet = FALSE)
37-
curl::curl_download(url_md5, dest_file_md5, quiet = TRUE)
38-
39-
if (!quiet) {
40-
cli::cli_inform("Download completed.", .envir = environment())
41-
}
42-
md5sum <- tools::md5sum(dest_file)
43-
md5sum_expected <- readLines(dest_file_md5, warn = FALSE)
44-
45-
if (md5sum != md5sum_expected) {
46-
cli::cli_abort(
47-
"MD5 checksum mismatch. Please try downloading the file again.",
48-
.envir = environment()
49-
)
50-
unlink(dest_file)
51-
return(NULL)
52-
} else {
53-
if (!quiet) {
54-
cli::cli_inform("MD5 checksum verified.", .envir = environment())
55-
}
56-
}
57-
}
58-
5934
# override cache_path if temp_dir is set to TRUE
6035
if (temp_dir) {
6136
temp_dir <- tempdir()
@@ -160,7 +135,39 @@ java_download <- function(
160135
file.remove(dest_file)
161136
}
162137

163-
download_dist_check_md5(url, dest_file, quiet)
138+
download_dist_with_md5_check(url, dest_file, url_md5, dest_file_md5, quiet)
164139

165140
return(dest_file)
166141
}
142+
143+
# Helper function to download a distribution and verify its MD5 checksum
144+
download_dist_with_md5_check <- function(
145+
url,
146+
dest_file,
147+
url_md5,
148+
dest_file_md5,
149+
quiet
150+
) {
151+
# Perform downloads
152+
curl::curl_download(url, dest_file, quiet = FALSE)
153+
curl::curl_download(url_md5, dest_file_md5, quiet = TRUE)
154+
155+
if (!quiet) {
156+
cli::cli_inform("Download completed.")
157+
}
158+
159+
# Verify checksum
160+
md5sum_actual <- tools::md5sum(dest_file)
161+
md5sum_expected <- readLines(dest_file_md5, warn = FALSE)
162+
163+
if (md5sum_actual != md5sum_expected) {
164+
unlink(dest_file) # Clean up failed download
165+
cli::cli_abort(
166+
"MD5 checksum mismatch. Please try downloading the file again."
167+
)
168+
}
169+
170+
if (!quiet) {
171+
cli::cli_inform("MD5 checksum verified.")
172+
}
173+
}

R/java_install.R

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#' Install Java from a distribution file
2-
#'
2+
#'
33
#' @description
44
#' Unpack Java distribution file into cache directory and link the installation into a project directory, optionally setting the `JAVA_HOME` and `PATH` environment variables to the Java version that was just installed.
55
#'
@@ -121,6 +121,7 @@ java_install <- function(
121121
unlink(project_version_path, recursive = TRUE)
122122
}
123123
file.symlink(installed_path, project_version_path)
124+
link_success <- TRUE # <--- THIS IS THE ONLY CHANGE
124125
},
125126
warning = function(w) {
126127
if (!quiet) cli::cli_inform("Warning: {w}")
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# This file is run automatically by testthat before tests are run.
2+
# It's a good place for mocks that are used across multiple test files.
3+
4+
# Mock the slow network call to java_valid_versions()
5+
local_mocked_bindings(
6+
java_valid_versions = function(...) c("8", "11", "17", "21")
7+
)
8+
9+
# Mock common dependencies for java_install() tests
10+
local_mocked_bindings(
11+
# Assume user consent is always given in tests
12+
rje_consent_check = function() TRUE,
13+
# Assume java_unpack always succeeds and returns a predictable path.
14+
# We are not testing java_unpack here.
15+
# CORRECTED: This mock is now self-contained and does not depend on global options.
16+
java_unpack = function(java_distrib_path, ...) {
17+
filename <- basename(java_distrib_path)
18+
parts <- strsplit(gsub("\\.tar\\.gz|\\.zip", "", filename), "-")[[1]]
19+
version <- parts[parts %in% c("8", "11", "17", "21")][1]
20+
arch <- parts[parts %in% c("x64", "aarch64")][1]
21+
platform <- parts[parts %in% c("linux", "windows", "macos")][1]
22+
23+
# Use a generic, hardcoded root path instead of calling getOption().
24+
# This makes the mock independent of the state being tested.
25+
file.path(
26+
"/mock/cache/path",
27+
"installed",
28+
platform,
29+
arch,
30+
version
31+
)
32+
}
33+
)
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
# Mocked unit tests for java_download()
2+
test_that("java_download handles successful download and checksum", {
3+
local_cache_path <- withr::local_tempdir()
4+
5+
# MOCK THE SLOW NETWORK CALL
6+
local_mocked_bindings(
7+
java_valid_versions = function(...) c("8", "11", "17", "21")
8+
)
9+
10+
local_mocked_bindings(
11+
curl_download = function(url, destfile, ...) {
12+
if (grepl("\\.md5$", destfile)) {
13+
writeLines("dummymd5checksum", destfile)
14+
} else {
15+
writeLines("dummy content", destfile)
16+
}
17+
},
18+
.package = "curl"
19+
)
20+
21+
local_mocked_bindings(
22+
md5sum = function(files) setNames("dummymd5checksum", files),
23+
.package = "tools"
24+
)
25+
26+
expect_silent(
27+
result_path <- java_download(
28+
version = "21",
29+
cache_path = local_cache_path,
30+
quiet = TRUE
31+
)
32+
)
33+
expect_true(file.exists(result_path))
34+
})
35+
36+
test_that("java_download aborts on MD5 checksum mismatch", {
37+
local_cache_path <- withr::local_tempdir()
38+
39+
# MOCK THE SLOW NETWORK CALL
40+
local_mocked_bindings(
41+
java_valid_versions = function(...) c("8", "11", "17", "21")
42+
)
43+
44+
local_mocked_bindings(
45+
curl_download = function(url, destfile, ...) {
46+
if (grepl("\\.md5$", destfile)) {
47+
writeLines("expected_checksum", destfile)
48+
} else {
49+
writeLines("some content", destfile)
50+
}
51+
},
52+
.package = "curl"
53+
)
54+
55+
local_mocked_bindings(
56+
md5sum = function(files) setNames("actual_checksum_mismatch", files),
57+
.package = "tools"
58+
)
59+
60+
expect_error(
61+
java_download(version = "21", cache_path = local_cache_path, quiet = TRUE),
62+
"MD5 checksum mismatch"
63+
)
64+
})
65+
66+
test_that("java_download skips download if file exists and force is FALSE", {
67+
local_cache_path <- withr::local_tempdir()
68+
dest_dir <- file.path(local_cache_path, "distrib")
69+
dir.create(dest_dir, recursive = TRUE)
70+
71+
# MOCK THE SLOW NETWORK CALL
72+
local_mocked_bindings(
73+
java_valid_versions = function(...) c("8", "11", "17", "21")
74+
)
75+
76+
platform <- platform_detect()$os
77+
arch <- platform_detect()$arch
78+
java_urls <- java_urls_load()
79+
url_template <- java_urls[["Corretto"]][[platform]][[arch]]
80+
url <- gsub("\\{version\\}", "21", url_template)
81+
expected_file_path <- file.path(dest_dir, basename(url))
82+
file.create(expected_file_path)
83+
84+
local_mocked_bindings(
85+
curl_download = function(...) {
86+
stop("curl_download should not have been called!")
87+
},
88+
.package = "curl"
89+
)
90+
91+
result <- java_download(
92+
version = "21",
93+
cache_path = local_cache_path,
94+
platform = platform,
95+
arch = arch,
96+
quiet = TRUE,
97+
force = FALSE
98+
)
99+
100+
expect_equal(normalizePath(result), normalizePath(expected_file_path))
101+
})
102+
103+
104+
test_that("java_download overwrites if file exists and force is TRUE", {
105+
local_cache_path <- withr::local_tempdir()
106+
dest_dir <- file.path(local_cache_path, "distrib")
107+
dir.create(dest_dir, recursive = TRUE)
108+
109+
# MOCK THE SLOW NETWORK CALL
110+
local_mocked_bindings(
111+
java_valid_versions = function(...) c("8", "11", "17", "21")
112+
)
113+
114+
platform <- platform_detect()$os
115+
arch <- platform_detect()$arch
116+
java_urls <- java_urls_load()
117+
url_template <- java_urls[["Corretto"]][[platform]][[arch]]
118+
url <- gsub("\\{version\\}", "21", url_template)
119+
expected_file_path <- file.path(dest_dir, basename(url))
120+
writeLines("old content", expected_file_path)
121+
122+
curl_call_count <- 0
123+
local_mocked_bindings(
124+
curl_download = function(url, destfile, ...) {
125+
curl_call_count <<- curl_call_count + 1
126+
if (grepl("\\.md5$", destfile)) {
127+
writeLines("new_checksum", destfile)
128+
} else {
129+
writeLines("new content", destfile)
130+
}
131+
},
132+
.package = "curl"
133+
)
134+
135+
local_mocked_bindings(
136+
md5sum = function(files) setNames("new_checksum", files),
137+
.package = "tools"
138+
)
139+
140+
java_download(
141+
version = "21",
142+
cache_path = local_cache_path,
143+
platform = platform,
144+
arch = arch,
145+
quiet = TRUE,
146+
force = TRUE
147+
)
148+
149+
expect_equal(curl_call_count, 2)
150+
expect_equal(readLines(expected_file_path), "new content")
151+
})

0 commit comments

Comments
 (0)