Skip to content

Commit 04a8ece

Browse files
committed
fix deparse bug and add regression test
1 parent b5fbd97 commit 04a8ece

2 files changed

Lines changed: 92 additions & 18 deletions

File tree

R/java_env.R

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -313,31 +313,32 @@ java_check_version_rjava <- function(
313313
)
314314
get_libjvm_path_fn <- getFromNamespace("get_libjvm_path", "rJavaEnv")
315315

316-
java_version_check_body <- paste(
317-
deparse(body(java_version_check_fn)),
318-
collapse = "\n"
319-
)
320-
get_libjvm_path_body <- paste(
321-
deparse(body(get_libjvm_path_fn)),
322-
collapse = "\n"
323-
)
316+
# Helper to deparse and collapse multi-line expressions
317+
deparse_collapse <- function(x) {
318+
paste(deparse(x, width.cutoff = 500), collapse = "\n")
319+
}
320+
321+
java_version_check_body <- deparse_collapse(body(java_version_check_fn))
322+
get_libjvm_path_body <- deparse_collapse(body(get_libjvm_path_fn))
323+
libs_val <- deparse_collapse(as.character(.libPaths()))
324324

325325
# Create a wrapper script that includes the function definitions and calls them
326326
# Capture current libPaths to ensure subprocess can find rJava in renv/packrat environments
327-
libs_code <- paste0(".libPaths(", deparse(as.character(.libPaths())), ")")
328-
329327
wrapper_script <- paste0(
330-
libs_code,
331-
"\n\n",
332-
"get_libjvm_path <- function(java_home) {\n",
328+
".libPaths(",
329+
libs_val,
330+
")\n\n",
331+
"get_libjvm_path <- function(java_home) ",
333332
get_libjvm_path_body,
334-
"\n}\n\n",
335-
"java_version_check <- function(java_home) {\n",
333+
"\n\n",
334+
"java_version_check <- function(java_home) ",
336335
java_version_check_body,
337-
"\n}\n\n",
336+
"\n\n",
338337
"args <- commandArgs(trailingOnly = TRUE)\n",
339-
"result <- java_version_check(args[1])\n",
340-
"cat(result, sep = '\n')"
338+
"if (length(args) > 0) {\n",
339+
" result <- java_version_check(args[1])\n",
340+
" cat(result, sep = '\n')\n",
341+
"}"
341342
)
342343

343344
# Write the wrapper script to a temporary file
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
#' Test that java_check_version_rjava() correctly constructs the subprocess script
2+
#' without duplication issues when .libPaths() contains multiple paths.
3+
4+
test_that("._java_version_check_rjava_impl_original handles multi-line .libPaths correctly", {
5+
# Mock a scenario where .libPaths() has multiple long paths
6+
# This previously caused duplication in paste0() because deparse() returned a vector.
7+
mock_paths <- c(
8+
"/usr/lib/R/library",
9+
"/usr/local/lib/R/site-library",
10+
"/home/user/R/x86_64-pc-linux-gnu-library/4.5"
11+
)
12+
13+
captured_script <- NULL
14+
15+
# Mock get_libjvm_path to return a simple known string
16+
local_mocked_bindings(
17+
get_libjvm_path = function(...) "/mock/libjvm.so",
18+
.package = "rJavaEnv"
19+
)
20+
21+
local_mocked_bindings(
22+
.libPaths = function(...) mock_paths,
23+
system2 = function(command, args, ...) {
24+
# The first argument in args is the script file path
25+
if (file.exists(args[1])) {
26+
captured_script <<- readLines(args[1])
27+
}
28+
# Return valid dummy output
29+
return(c(
30+
"rJava and other rJava/Java-based packages will use Java version: \"21\""
31+
))
32+
},
33+
.package = "base"
34+
)
35+
36+
# Call the internal function
37+
result <- rJavaEnv:::._java_version_check_rjava_impl_original(
38+
java_home = "/mock/java"
39+
)
40+
41+
# Assertions on the captured script
42+
# Avoid expect_* functions that might trigger 'waldo' dependency issues.
43+
# Using simple R logic that throws an error on failure, which testthat will catch.
44+
if (is.null(captured_script)) {
45+
stop("Captured script is NULL")
46+
}
47+
48+
# 1. Check for duplication of function definitions
49+
def_count <- sum(grepl("java_version_check <- function", captured_script))
50+
if (!identical(as.numeric(def_count), 1)) {
51+
stop(sprintf(
52+
"The function definition should appear exactly once, but found %d",
53+
def_count
54+
))
55+
}
56+
57+
# 2. Check that .libPaths() contains all paths in a valid call
58+
script_text <- paste(captured_script, collapse = "\n")
59+
if (!grepl("\\.libPaths\\(c\\(", script_text)) {
60+
stop("Should use c() for multiple library paths.")
61+
}
62+
if (!grepl("/home/user/R/x86_64-pc-linux-gnu-library/4.5", script_text)) {
63+
stop("Last path should be present.")
64+
}
65+
66+
# 3. Check for syntax errors (no stray commas at line ends from bad paste)
67+
if (grepl(", \\)", script_text)) {
68+
stop("Malformed closing parenthesis found.")
69+
}
70+
71+
# Final verification: if we reached here, the test passed.
72+
expect_silent(NULL)
73+
})

0 commit comments

Comments
 (0)