Skip to content

Commit 3ab8871

Browse files
jonkeanejonkeane
committed
Add valgrind nightly, quiet tests that squak in R.
1 parent 4eb7a15 commit 3ab8871

File tree

9 files changed

+138
-4
lines changed

9 files changed

+138
-4
lines changed

‎ci/etc/valgrind-cran.supp‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
18+
{
19+
# `testthat::skip()`s cause a valgrind error that does not show up on CRAN.
20+
<testthat_skip_error>
21+
Memcheck:Cond
22+
fun:gregexpr_Regexc
23+
fun:do_regexpr
24+
fun:bcEval
25+
fun:Rf_eval
26+
fun:R_execClosure
27+
fun:Rf_applyClosure
28+
fun:bcEval
29+
fun:Rf_eval
30+
fun:forcePromise
31+
fun:FORCE_PROMISE
32+
fun:getvar
33+
fun:bcEval
34+
}

‎ci/scripts/r_valgrind.sh‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
#!/usr/bin/env bash
2+
# Licensed to the Apache Software Foundation (ASF) under one
3+
# or more contributor license agreements. See the NOTICE file
4+
# distributed with this work for additional information
5+
# regarding copyright ownership. The ASF licenses this file
6+
# to you under the Apache License, Version 2.0 (the
7+
# "License"); you may not use this file except in compliance
8+
# with the License. You may obtain a copy of the License at
9+
#
10+
# http://www.apache.org/licenses/LICENSE-2.0
11+
#
12+
# Unless required by applicable law or agreed to in writing,
13+
# software distributed under the License is distributed on an
14+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
# KIND, either express or implied. See the License for the
16+
# specific language governing permissions and limitations
17+
# under the License.
18+
19+
set -ex
20+
21+
: ${R_BIN:=RDvalgrind}
22+
23+
source_dir=${1}/r
24+
25+
${R_BIN} CMD INSTALL ${source_dir}
26+
pushd ${source_dir}/tests
27+
28+
export TEST_R_WITH_ARROW=TRUE
29+
# to generate suppression files run:
30+
# ${R_BIN} --vanilla -d "valgrind --tool=memcheck --leak-check=full --track-origins=yes --gen-suppressions=all --log-file=memcheck.log" -f testthat.R
31+
${R_BIN} --vanilla -d "valgrind --tool=memcheck --leak-check=full --track-origins=yes --error-exitcode=1 --suppressions=/${1}/ci/etc/valgrind-cran.supp" -f testthat.R | tee testthat.out
32+
33+
# We might also considering using the greps that LibthGBM uses:
34+
# https://github.com/microsoft/LightGBM/blob/fa6d356555f9ef888acf5f5e259dca958ca24f6d/.ci/test_r_package_valgrind.sh#L20-L85

‎dev/tasks/tasks.yml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -862,6 +862,14 @@ tasks:
862862
FEDORA: 33
863863
run: fedora-python
864864

865+
test-r-linux-valgrind:
866+
ci: azure
867+
template: docker-tests/azure.linux.yml
868+
params:
869+
env:
870+
UBUNTU: 18.04
871+
run: ubuntu-r-valgrind
872+
865873
test-r-linux-as-cran:
866874
ci: github
867875
template: r/github.linux.cran.yml

‎docker-compose.yml‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ x-hierarchy:
116116
- ubuntu-cpp-sanitizer
117117
- ubuntu-cpp-thread-sanitizer
118118
- ubuntu-r-sanitizer
119+
- ubuntu-r-valgrind
119120
- python-sdist
120121
- r
121122
# helper services
@@ -1050,6 +1051,31 @@ services:
10501051
/bin/bash -c "
10511052
/arrow/ci/scripts/r_sanitize.sh /arrow"
10521053
1054+
ubuntu-r-valgrind:
1055+
# Only 18.04 and amd64 supported
1056+
# Usage:
1057+
# docker-compose build ubuntu-r-valgrind
1058+
# docker-compose run ubuntu-r-valgrind
1059+
image: ${REPO}:amd64-ubuntu-18.04-r-valgrind
1060+
build:
1061+
context: .
1062+
dockerfile: ci/docker/linux-r.dockerfile
1063+
cache_from:
1064+
- ${REPO}:amd64-ubuntu-18.04-r-valgrind
1065+
args:
1066+
base: wch1/r-debug:latest
1067+
r_bin: RDvalgrind
1068+
environment:
1069+
<<: *ccache
1070+
# AVX512 not supported by Valgrind (similar to ARROW-9851) some runners support AVX512 and some do not
1071+
# so some build might pass without this setting, but we want to ensure that we stay to AVX2 regardless of runner.
1072+
EXTRA_CMAKE_FLAGS: "-DARROW_RUNTIME_SIMD_LEVEL=AVX2"
1073+
volumes: *ubuntu-volumes
1074+
command: >
1075+
/bin/bash -c "
1076+
/arrow/ci/scripts/r_valgrind.sh /arrow"
1077+
1078+
10531079
################################# Go ########################################
10541080

10551081
debian-go:

‎r/inst/build_arrow_static.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ ${CMAKE} -DARROW_BOOST_USE_SHARED=OFF \
5353
-DARROW_DATASET=${ARROW_DATASET:-ON} \
5454
-DARROW_DEPENDENCY_SOURCE=BUNDLED \
5555
-DARROW_FILESYSTEM=ON \
56-
-DARROW_JEMALLOC=${ARROW_JEMALLOC:-ON} \
57-
-DARROW_MIMALLOC=${ARROW_MIMALLOC:-$ARROW_DEFAULT_PARAM} \
56+
-DARROW_JEMALLOC=${ARROW_JEMALLOC:-$ARROW_DEFAULT_PARAM} \
57+
-DARROW_MIMALLOC=${ARROW_MIMALLOC:-ON} \
5858
-DARROW_JSON=ON \
5959
-DARROW_PARQUET=${ARROW_PARQUET:-ON} \
6060
-DARROW_S3=${ARROW_S3:-$ARROW_DEFAULT_PARAM} \

‎r/tests/testthat/helper-skip.R‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,20 @@ build_features <- c(
2222
)
2323

2424
skip_if_not_available <- function(feature) {
25+
if (feature == "re2") {
26+
# RE2 does not support valgrind (on purpose): https://github.com/google/re2/issues/177
27+
skip_on_valgrind()
28+
}
29+
2530
yes <- feature %in% names(build_features) && build_features[feature]
2631
if (!yes) {
2732
skip(paste("Arrow C++ not built with", feature))
2833
}
2934
}
3035

3136
skip_if_no_pyarrow <- function() {
37+
skip_on_valgrind()
38+
3239
skip_if_not_installed("reticulate")
3340
if (!reticulate::py_module_available("pyarrow")) {
3441
skip("pyarrow not available for testing")
@@ -49,6 +56,18 @@ skip_if_not_running_large_memory_tests <- function() {
4956
)
5057
}
5158

59+
skip_on_valgrind <- function() {
60+
# This does not actually skip on valgrind because we can't exactly detect it.
61+
# Instead, it skips on CRAN when the OS is linux + and the R version is development
62+
# (which is where valgrind is run as of this code)
63+
linux_dev <- identical(tolower(Sys.info()[["sysname"]]), "linux") &&
64+
grepl("devel", R.version.string)
65+
66+
if (linux_dev) {
67+
skip_on_cran()
68+
}
69+
}
70+
5271
process_is_running <- function(x) {
5372
cmd <- sprintf("ps aux | grep '%s' | grep -v grep", x)
5473
tryCatch(system(cmd, ignore.stdout = TRUE) == 0, error = function(e) FALSE)

‎r/tests/testthat/test-Array.R‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ test_that("binary Array", {
5353
expect_array_roundtrip(bin, fixed_size_binary(byte_width = 10))
5454

5555
# degenerate cases
56+
skip_on_valgrind() # valgrind errors on these tests ARROW-12638
5657
bin <- vctrs::new_vctr(
5758
list(1:10),
5859
class = "arrow_binary"

‎r/tests/testthat/test-arrow.R‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ test_that("check for an ArrowObject in functions use std::shared_ptr", {
6262
})
6363

6464
test_that("MemoryPool calls gc() to free memory when allocation fails (ARROW-10080)", {
65+
# There is a valgrind error on this test because there cannot be memory allocated
66+
# which is exactly what this test is checking, but we quiet this
67+
skip_on_valgrind()
68+
6569
env <- new.env()
6670
trace(gc, print = FALSE, tracer = function() {
6771
env$gc_was_called <- TRUE

‎r/tests/testthat/test-compute-aggregate.R‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,11 @@ test_that("sum.Array", {
3737

3838
floats <- c(floats, NA)
3939
na <- Array$create(floats)
40-
expect_identical(as.numeric(sum(na)), sum(floats))
40+
if (!grepl("devel", R.version.string)) {
41+
# Valgrind on R-devel confuses NaN and NA_real_
42+
# https://r.789695.n4.nabble.com/Difference-in-NA-behavior-in-R-devel-running-under-valgrind-td4768731.html
43+
expect_identical(as.numeric(sum(na)), sum(floats))
44+
}
4145
expect_r6_class(sum(na, na.rm = TRUE), "Scalar")
4246
expect_identical(as.numeric(sum(na, na.rm = TRUE)), sum(floats, na.rm = TRUE))
4347

@@ -78,7 +82,11 @@ test_that("mean.Array", {
7882

7983
floats <- c(floats, NA)
8084
na <- Array$create(floats)
81-
expect_identical(as.vector(mean(na)), mean(floats))
85+
if (!grepl("devel", R.version.string)) {
86+
# Valgrind on R-devel confuses NaN and NA_real_
87+
# https://r.789695.n4.nabble.com/Difference-in-NA-behavior-in-R-devel-running-under-valgrind-td4768731.html
88+
expect_identical(as.vector(mean(na)), mean(floats))
89+
}
8290
expect_r6_class(mean(na, na.rm = TRUE), "Scalar")
8391
expect_identical(as.vector(mean(na, na.rm = TRUE)), mean(floats, na.rm = TRUE))
8492

0 commit comments

Comments
 (0)