Skip to content

Commit fa56551

Browse files
e-kotove-kotov
authored
Revise live download tests (#89)
* chore: Add common cache and config directories to .gitignore. * feat: Dynamically load `libjvm.so` on Linux to ensure proper Java environment initialization. * refactor: Replace `requireNamespace` with `find.package` for `rJava` installation check. * Update .Rbuildignore, refactor GitHub workflow for R CMD check, and enhance Java live test with configurable quiet modes and improved verification steps * Refactored global mocks to helper-mocks.R and updated tests to use mock_java_globals() for improved test isolation.
1 parent 2e7f1b9 commit fa56551

6 files changed

Lines changed: 137 additions & 38 deletions

File tree

‎.Rbuildignore‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,7 @@
2727
^tools$
2828
^\.editorconfig$
2929
^revdep$
30+
^\.cache$
31+
^\.config$
32+
^\.local$
33+
^\.jupyter$

‎R/internal_utilities.R‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,46 @@ java_version_check_rscript <- function(java_home) {
128128
new_path <- file.path(java_home, "bin")
129129
Sys.setenv(PATH = paste(new_path, old_path, sep = .Platform$path.sep))
130130

131+
# On Linux, find and dynamically load libjvm.so
132+
if (Sys.info()["sysname"] == "Linux") {
133+
all_files <- list.files(
134+
path = java_home,
135+
pattern = "libjvm.so$",
136+
recursive = TRUE,
137+
full.names = TRUE
138+
)
139+
140+
libjvm_path <- NULL
141+
if (length(all_files) > 0) {
142+
# Prefer the 'server' version if available
143+
server_files <- all_files[grepl("/server/libjvm.so$", all_files)]
144+
if (length(server_files) > 0) {
145+
libjvm_path <- server_files[1]
146+
} else {
147+
libjvm_path <- all_files[1]
148+
}
149+
}
150+
151+
if (!is.null(libjvm_path) && file.exists(libjvm_path)) {
152+
tryCatch(
153+
dyn.load(libjvm_path),
154+
error = function(e) {
155+
# Use base message to avoid dependency issues in the isolated script
156+
message(sprintf(
157+
"Found libjvm.so at '%s' but failed to load it: %s",
158+
libjvm_path,
159+
e$message
160+
))
161+
}
162+
)
163+
} else {
164+
message(sprintf(
165+
"Could not find libjvm.so within the provided JAVA_HOME: %s",
166+
java_home
167+
))
168+
}
169+
}
170+
131171
suppressWarnings(rJava::.jinit())
132172
suppressWarnings(
133173
java_version <- rJava::.jcall(

‎R/java_env.R‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ java_check_version_rjava <- function(
242242
quiet = FALSE
243243
) {
244244
# Check if rJava is installed
245-
if (!requireNamespace("rJava", quietly = TRUE)) {
245+
if (length(find.package("rJava", quiet = TRUE)) == 0) {
246246
cli::cli_alert_danger(
247247
"rJava package is not installed. You need to install rJava to use this function to check if rJava-based packages will work with the specified Java version."
248248
)

‎tests/testthat/helper-mocks.R‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
# Helper function to mock global dependencies for java_install tests
2+
mock_java_globals <- function(env = parent.frame()) {
3+
# We use assignInNamespace because local_mocked_bindings was failing to mock
4+
# the functions in the package namespace reliably in this context.
5+
6+
# Mock java_valid_versions
7+
# It is internal, but assignInNamespace handles it.
8+
if (exists("java_valid_versions", envir = asNamespace("rJavaEnv"))) {
9+
original_java_valid_versions <- get(
10+
"java_valid_versions",
11+
envir = asNamespace("rJavaEnv")
12+
)
13+
assignInNamespace(
14+
"java_valid_versions",
15+
function(...) c("8", "11", "17", "21"),
16+
ns = "rJavaEnv"
17+
)
18+
withr::defer(
19+
assignInNamespace(
20+
"java_valid_versions",
21+
original_java_valid_versions,
22+
ns = "rJavaEnv"
23+
),
24+
envir = env
25+
)
26+
}
27+
28+
# Mock rje_consent_check
29+
if (exists("rje_consent_check", envir = asNamespace("rJavaEnv"))) {
30+
original_rje_consent_check <- get(
31+
"rje_consent_check",
32+
envir = asNamespace("rJavaEnv")
33+
)
34+
assignInNamespace("rje_consent_check", function() TRUE, ns = "rJavaEnv")
35+
withr::defer(
36+
assignInNamespace(
37+
"rje_consent_check",
38+
original_rje_consent_check,
39+
ns = "rJavaEnv"
40+
),
41+
envir = env
42+
)
43+
}
44+
45+
# Mock java_unpack
46+
if (exists("java_unpack", envir = asNamespace("rJavaEnv"))) {
47+
original_java_unpack <- get("java_unpack", envir = asNamespace("rJavaEnv"))
48+
49+
mock_unpack <- function(java_distrib_path, ...) {
50+
filename <- basename(java_distrib_path)
51+
parts <- strsplit(gsub("\\.tar\\.gz|\\.zip", "", filename), "-")[[1]]
52+
version <- parts[parts %in% c("8", "11", "17", "21")][1]
53+
arch <- parts[parts %in% c("x64", "aarch64")][1]
54+
platform <- parts[parts %in% c("linux", "windows", "macos")][1]
55+
56+
# Use a generic, hardcoded root path instead of calling getOption().
57+
# This makes the mock independent of the state being tested.
58+
file.path(
59+
"/mock/cache/path",
60+
"installed",
61+
platform,
62+
arch,
63+
version
64+
)
65+
}
66+
67+
assignInNamespace("java_unpack", mock_unpack, ns = "rJavaEnv")
68+
withr::defer(
69+
assignInNamespace("java_unpack", original_java_unpack, ns = "rJavaEnv"),
70+
envir = env
71+
)
72+
}
73+
}

‎tests/testthat/setup-mock-globals.R‎

Lines changed: 4 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,7 @@
11
# This file is run automatically by testthat before tests are run.
22
# It's a good place for mocks that are used across multiple test files.
33

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-
)
4+
# NOTE: Mocks defined here with local_mocked_bindings() will NOT persist
5+
# into the test files because the scope ends when this file finishes sourcing.
6+
# Instead, we use helper-mocks.R which defines functions to apply mocks
7+
# within the test scopes.

‎tests/testthat/test-java_install-mocked.R‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,19 @@ test_that("java_install succeeds with symlink on Unix-like systems", {
2424
# 3. CRITICAL: Create the empty dummy file for untar to find.
2525
file.create(fake_distrib_path)
2626

27+
# Apply the mocks
28+
mock_java_globals()
29+
2730
# The global mock will generate the unpacked path based on the distrib path
28-
fake_unpacked_path <- java_unpack(fake_distrib_path)
31+
# Since we mocked java_unpack in the namespace but the attached version might be stale,
32+
# we manually construct the expected path to match the mock's logic.
33+
fake_unpacked_path <- file.path(
34+
"/mock/cache/path",
35+
"installed",
36+
os,
37+
arch,
38+
"21"
39+
)
2940
expected_symlink_path <- file.path(
3041
local_proj_path,
3142
"rjavaenv",
@@ -85,6 +96,7 @@ test_that("java_install falls back to file.copy when symlink fails on Unix", {
8596
fake_distrib_path <- file.path(distrib_dir, fake_filename)
8697
file.create(fake_distrib_path)
8798

99+
mock_java_globals()
88100
local_mocked_bindings(java_env_set = function(...) TRUE)
89101

90102
local_mocked_bindings(
@@ -122,12 +134,7 @@ test_that("java_install respects autoset_java_env = FALSE", {
122134
# Add a local mock for java_unpack(). This prevents the real function
123135
# (and its call to utils::untar) from ever running.
124136
# This isolates the test to only check the logic we care about.
125-
local_mocked_bindings(
126-
java_unpack = function(...) {
127-
# It just needs to return a plausible-looking path string.
128-
"/mocked/unpacked/path"
129-
}
130-
)
137+
mock_java_globals()
131138

132139
# Mock file.symlink to prevent another potential side-effect
133140
local_mocked_bindings(
@@ -171,6 +178,7 @@ test_that("java_install succeeds with mklink junction on Windows", {
171178
)
172179
# No need to create the file, as it doesn't call untar/unzip.
173180

181+
mock_java_globals()
174182
local_mocked_bindings(java_env_set = function(...) TRUE)
175183

176184
system2_args <- list()

0 commit comments

Comments
 (0)