From ce69031d512ed120d25d4c77905c67ddff5d0d1a Mon Sep 17 00:00:00 2001 From: Jonathan Keane Date: Mon, 6 Nov 2023 19:08:13 -0600 Subject: [PATCH] GH-38570: [R] Ensure that test-nix-libs is warning free (#38571) ### Rationale for this change Although we stop / fail the CI job for warnings elsewhere, we don't seem to be doing that for our buildsystem tests. Let's change that so warnings + other output doesn't creep in. - [x] Prevent `*** Failed to find latest nightly for 8.0.0.9000` from showing up when the file is sourced under test - [x] fix `Warning ('test-nixlibs.R:140:3'): select_binary() with test program 'x' is NULL so the result will be NULL` - [x] update version strings to be characters to avoid r-devel warnings ### What changes are included in this PR? Added `stop_on_warning` to the test call, and fixes to make that pass ### Are these changes tested? Yes, they are all part of the test suite ### Are there any user-facing changes? No. * Closes: #38570 Authored-by: Jonathan Keane Signed-off-by: Jacob Wujciak-Jens --- ci/scripts/r_test.sh | 2 +- r/tools/check-versions.R | 4 ++-- r/tools/nixlibs.R | 14 +++++++++----- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/ci/scripts/r_test.sh b/ci/scripts/r_test.sh index e0c2ce9efedd8..22ec551edb9fa 100755 --- a/ci/scripts/r_test.sh +++ b/ci/scripts/r_test.sh @@ -27,7 +27,7 @@ pushd ${source_dir} printenv # Run the nixlibs.R test suite, which is not included in the installed package -${R_BIN} -e 'setwd("tools"); testthat::test_dir(".")' +${R_BIN} -e 'setwd("tools"); testthat::test_dir(".", stop_on_warning = TRUE)' # Before release, we always copy the relevant parts of the cpp source into the # package. In some CI checks, we will use this version of the source: diff --git a/r/tools/check-versions.R b/r/tools/check-versions.R index 45fbd0b61a40a..3d8cbf02a14c9 100644 --- a/r/tools/check-versions.R +++ b/r/tools/check-versions.R @@ -23,8 +23,8 @@ test_mode <- exists("TESTING") check_versions <- function(r_version, cpp_version) { r_parsed <- package_version(r_version) r_dev_version <- r_parsed[1, 4] - r_is_dev <- !is.na(r_dev_version) && r_dev_version > 100 - r_is_patch <- !is.na(r_dev_version) && r_dev_version <= 100 + r_is_dev <- !is.na(r_dev_version) && r_dev_version > "100" + r_is_patch <- !is.na(r_dev_version) && r_dev_version <= "100" cpp_is_dev <- grepl("SNAPSHOT$", cpp_version) cpp_parsed <- package_version(sub("-SNAPSHOT$", "", cpp_version)) diff --git a/r/tools/nixlibs.R b/r/tools/nixlibs.R index 96f2b858cf180..63c185ce54563 100644 --- a/r/tools/nixlibs.R +++ b/r/tools/nixlibs.R @@ -198,7 +198,11 @@ select_binary <- function(os = tolower(Sys.info()[["sysname"]]), errs <- compile_test_program(test_program) openssl_version <- determine_binary_from_stderr(errs) arch <- ifelse(identical(os, "darwin"), paste0("-", arch, "-"), "-") - ifelse(is.null(openssl_version), NULL, paste0(os, arch, openssl_version)) + if (is.null(openssl_version)) { + NULL + } else { + paste0(os, arch, openssl_version) + } }, error = function(e) { lg("Unable to find libcurl and openssl") @@ -210,7 +214,7 @@ select_binary <- function(os = tolower(Sys.info()[["sysname"]]), lg("Building on %s %s", os, arch) binary <- NULL } - return(binary) + binary } # This tests that curl and OpenSSL are present (bc we can include their headers) @@ -268,7 +272,7 @@ get_macos_openssl_dir <- function() { openssl_root_dir <- "/usr/local" } } - return(openssl_root_dir) + openssl_root_dir } # (built with newer devtoolset but older glibc (2.17) for broader compatibility,# like manylinux2014) @@ -828,10 +832,10 @@ quietly <- !env_is("ARROW_R_DEV", "true") not_cran <- env_is("NOT_CRAN", "true") -if (is_release) { +if (is_release & !test_mode) { VERSION <- VERSION[1, 1:3] arrow_repo <- paste0(getOption("arrow.repo", sprintf("https://apache.jfrog.io/artifactory/arrow/r/%s", VERSION)), "/libarrow/") -} else { +} else if(!test_mode) { not_cran <- TRUE arrow_repo <- paste0(getOption("arrow.dev_repo", "https://nightlies.apache.org/arrow/r"), "/libarrow/") VERSION <- find_latest_nightly(VERSION)