GH-50934: [C++][Dev] Fix shellcheck errors in cpp/build-support/run-test.sh by hiroyuki-sato · Pull Request #50935 · apache/arrow

hiroyuki-sato · 2026-08-21T00:58:48Z

Rationale for this change

This is the sub issue #44748.

SC2010: Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.
SC2035: Use ./glob or -- glob so names with dashes won't become options.
SC2046: Quote this to prevent word splitting.
SC2048: Use "$@" (with quotes) to prevent whitespace problems.
SC2086: Double quote to prevent globbing and word splitting.
SC2128: Expanding an array without an index only gives the element in the index 0.
SC2164: Use cd ... || exit in case cd fails.
SC2209: Use var=$(command) to assign output (or quote to assign string).


In cpp/build-support/run-test.sh line 28:
ROOT=$(cd $(dirname $BASH_SOURCE)/..; pwd)
       ^---------------------------^ SC2164 (warning): Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
          ^---------------------^ SC2046 (warning): Quote this to prevent word splitting.
                    ^----------^ SC2128 (warning): Expanding an array without an index only gives the first element.
                    ^----------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
ROOT=$(cd $(dirname "$BASH_SOURCE")/.. || exit; pwd)


In cpp/build-support/run-test.sh line 31:
mkdir -p $TEST_LOGDIR
         ^----------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
mkdir -p "$TEST_LOGDIR"


In cpp/build-support/run-test.sh line 36:
mkdir -p $TEST_DEBUGDIR
         ^------------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
mkdir -p "$TEST_DEBUGDIR"


In cpp/build-support/run-test.sh line 38:
TEST_DIRNAME=$(cd $(dirname $1); pwd)
               ^--------------^ SC2164 (warning): Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
                  ^-----------^ SC2046 (warning): Quote this to prevent word splitting.
                            ^-- SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
TEST_DIRNAME=$(cd $(dirname "$1") || exit; pwd)


In cpp/build-support/run-test.sh line 39:
TEST_FILENAME=$(basename $1)
                         ^-- SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
TEST_FILENAME=$(basename "$1")


In cpp/build-support/run-test.sh line 42:
TEST_NAME=$(echo $TEST_FILENAME | sed -E -e 's/\..+$//') # Remove path and extension (if any).
                 ^------------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
TEST_NAME=$(echo "$TEST_FILENAME" | sed -E -e 's/\..+$//') # Remove path and extension (if any).


In cpp/build-support/run-test.sh line 46:
mkdir -p $TEST_WORKDIR
         ^-----------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
mkdir -p "$TEST_WORKDIR"


In cpp/build-support/run-test.sh line 47:
pushd $TEST_WORKDIR >/dev/null || exit 1
      ^-----------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
pushd "$TEST_WORKDIR" >/dev/null || exit 1


In cpp/build-support/run-test.sh line 48:
rm -f *
      ^-- SC2035 (info): Use ./*glob* or -- *glob* so names with dashes won't become options.


In cpp/build-support/run-test.sh line 59:
rm -f $LOGFILE $LOGFILE.gz
      ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.
               ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
rm -f "$LOGFILE" "$LOGFILE".gz


In cpp/build-support/run-test.sh line 61:
pipe_cmd=cat
^------^ SC2209 (warning): Use var=$(command) to assign output (or quote to assign string).


In cpp/build-support/run-test.sh line 93:
  rm -f $XMLFILE
        ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
  rm -f "$XMLFILE"


In cpp/build-support/run-test.sh line 95:
  $TEST_EXECUTABLE "$@" > $LOGFILE.raw 2>&1
                          ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
  $TEST_EXECUTABLE "$@" > "$LOGFILE".raw 2>&1


In cpp/build-support/run-test.sh line 97:
  cat $LOGFILE.raw \
      ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
  cat "$LOGFILE".raw \


In cpp/build-support/run-test.sh line 98:
    | ${PYTHON:-python} $ROOT/build-support/asan_symbolize.py \
                        ^---^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
    | ${PYTHON:-python} "$ROOT"/build-support/asan_symbolize.py \


In cpp/build-support/run-test.sh line 100:
    | $pipe_cmd 2>&1 | tee $LOGFILE
                           ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
    | $pipe_cmd 2>&1 | tee "$LOGFILE"


In cpp/build-support/run-test.sh line 101:
  rm -f $LOGFILE.raw
        ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
  rm -f "$LOGFILE".raw


In cpp/build-support/run-test.sh line 111:
  if grep -E -q "ThreadSanitizer|Leak check.*detected leaks" $LOGFILE ; then
                                                             ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
  if grep -E -q "ThreadSanitizer|Leak check.*detected leaks" "$LOGFILE" ; then


In cpp/build-support/run-test.sh line 112:
    echo ThreadSanitizer or leak check failures in $LOGFILE
                                                   ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
    echo ThreadSanitizer or leak check failures in "$LOGFILE"


In cpp/build-support/run-test.sh line 114:
    rm -f $XMLFILE
          ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
    rm -f "$XMLFILE"


In cpp/build-support/run-test.sh line 137:
  FILENAME=$(echo ${FILENAME} | cut -c-15)
                  ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
  FILENAME=$(echo "${FILENAME}" | cut -c-15)


In cpp/build-support/run-test.sh line 140:
  COREFILES=$(ls /tmp | grep $PATTERN)
              ^-- SC2010 (warning): Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.
                             ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
  COREFILES=$(ls /tmp | grep "$PATTERN")


In cpp/build-support/run-test.sh line 150:
        gdb -c "${COREPATH}" $TEST_EXECUTABLE -ex "thread apply all bt" -ex "set pagination 0" -batch
                             ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
        gdb -c "${COREPATH}" "$TEST_EXECUTABLE" -ex "thread apply all bt" -ex "set pagination 0" -batch


In cpp/build-support/run-test.sh line 163:
  if grep -E -q "ERROR: LeakSanitizer: detected memory leaks" $LOGFILE ; then
                                                              ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
  if grep -E -q "ERROR: LeakSanitizer: detected memory leaks" "$LOGFILE" ; then


In cpp/build-support/run-test.sh line 171:
      $XMLFILE
      ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
      "$XMLFILE"


In cpp/build-support/run-test.sh line 172:
    mv $XMLFILE.bak $XMLFILE
       ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.
                    ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
    mv "$XMLFILE".bak "$XMLFILE"


In cpp/build-support/run-test.sh line 178:
  $TEST_EXECUTABLE "$@" 2>&1 | $pipe_cmd > $LOGFILE
                                           ^------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
  $TEST_EXECUTABLE "$@" 2>&1 | $pipe_cmd > "$LOGFILE"


In cpp/build-support/run-test.sh line 182:
if [ $RUN_TYPE = "test" ]; then
     ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
if [ "$RUN_TYPE" = "test" ]; then


In cpp/build-support/run-test.sh line 188:
  if [ $ATTEMPT_NUMBER -lt $TEST_EXECUTION_ATTEMPTS ]; then
       ^-------------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
  if [ "$ATTEMPT_NUMBER" -lt $TEST_EXECUTION_ATTEMPTS ]; then


In cpp/build-support/run-test.sh line 195:
    TEST_TMPDIR_BEFORE=$(find $TEST_TMPDIR -maxdepth 1 -type d | sort)
                              ^----------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
    TEST_TMPDIR_BEFORE=$(find "$TEST_TMPDIR" -maxdepth 1 -type d | sort)


In cpp/build-support/run-test.sh line 198:
  if [ $ATTEMPT_NUMBER -lt $TEST_EXECUTION_ATTEMPTS ]; then
       ^-------------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
  if [ "$ATTEMPT_NUMBER" -lt $TEST_EXECUTION_ATTEMPTS ]; then


In cpp/build-support/run-test.sh line 200:
    TEST_TMPDIR_AFTER=$(find $TEST_TMPDIR -maxdepth 1 -type d | sort)
                             ^----------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
    TEST_TMPDIR_AFTER=$(find "$TEST_TMPDIR" -maxdepth 1 -type d | sort)


In cpp/build-support/run-test.sh line 218:
  if [ $RUN_TYPE = "test" ]; then
       ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
  if [ "$RUN_TYPE" = "test" ]; then


In cpp/build-support/run-test.sh line 219:
    run_test $*
             ^-- SC2048 (warning): Use "$@" (with quotes) to prevent whitespace problems.
             ^-- SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
    run_test "$*"


In cpp/build-support/run-test.sh line 221:
    run_other $*
              ^-- SC2048 (warning): Use "$@" (with quotes) to prevent whitespace problems.
              ^-- SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
    run_other "$*"


In cpp/build-support/run-test.sh line 226:
    echo Test failed attempt number $ATTEMPT_NUMBER
                                    ^-------------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
    echo Test failed attempt number "$ATTEMPT_NUMBER"


In cpp/build-support/run-test.sh line 231:
if [ $RUN_TYPE = "test" ]; then
     ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
if [ "$RUN_TYPE" = "test" ]; then


In cpp/build-support/run-test.sh line 237:
popd
^--^ SC2164 (warning): Use 'popd ... || exit' or 'popd ... || return' in case popd fails.

Did you mean:
popd || exit


In cpp/build-support/run-test.sh line 238:
rm -Rf $TEST_WORKDIR
       ^-----------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
rm -Rf "$TEST_WORKDIR"


In cpp/build-support/run-test.sh line 240:
exit $STATUS
     ^-----^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean:
exit "$STATUS"

For more information:
  https://www.shellcheck.net/wiki/SC2010 -- Don't use ls | grep. Use a glob o...
  https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...
  https://www.shellcheck.net/wiki/SC2048 -- Use "$@" (with quotes) to prevent...

What changes are included in this PR?

SC2010: Replace ls | grep with find
SC2035: Prefix globs with ./ to prevent filenames from being treated as options
SC2046: Quote command substitutions to prevent word splitting
SC2048: Use "$@" to preserve arguments containing whitespace
SC2086: Quote variables to prevent word splitting and globbing
SC2128: Specify the array index explicitly
SC2164: Handle cd failures explicitly
SC2209: Use command substitution when assigning command output

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

GitHub Issue: [C++] Fix shellcheck errors in cpp/build-support/run-test.sh #50934

github-actions · 2026-08-21T00:59:19Z

⚠️ GitHub issue #50934 has been automatically assigned in GitHub to PR creator.

hiroyuki-sato · 2026-08-23T08:31:22Z

rebased for re-run CI job

hiroyuki-sato · 2026-08-23T15:25:40Z

@kou I don't think the two CI failures are related to this PR. What do you think?

tadeja · 2026-08-23T22:37:37Z

Failing C++ / AMD64 Conda C++ AVX2 is the new issue #50930
(Re-ran ARM64 macOS 14 C++ and that one is passing now.)

hiroyuki-sato · 2026-08-24T00:16:07Z

@kou Thank you for re-run job. Please take a look this PR when you get a chance.
@tadeja Thank you for your comment.

kou · 2026-08-24T00:56:38Z

+ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) || {
+  echo "Failed to determine project root directory" >&2
+  exit 1
+}


Hmm. Can we use set -e instead?

I also tried using set -e instead of || { echo "error"; }.
However, it seems we need to modify more code.

For example, this part causes the script to exit when the exit status is 1.
I'm not sure if this is the exact reason, but it seems to cause an error.

Do you have any idea?

"$TEST_EXECUTABLE" "$@" > "${LOGFILE}.raw" 2>&1

diff --git a/cpp/build-support/run-test.sh b/cpp/build-support/run-test.sh index e3c0738410..0e02b1ab8a 100755 --- a/cpp/build-support/run-test.sh +++ b/cpp/build-support/run-test.sh @@ -23,12 +23,11 @@ # $ARGN - arguments for executable # +set -e + OUTPUT_ROOT="$1" shift -ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) || { - echo "Failed to determine project root directory" >&2 - exit 1 -} +ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) TEST_LOGDIR="$OUTPUT_ROOT/build/$1-logs" mkdir -p "$TEST_LOGDIR" @@ -38,10 +37,7 @@ shift TEST_DEBUGDIR="$OUTPUT_ROOT/build/$RUN_TYPE-debug" mkdir -p "$TEST_DEBUGDIR" -TEST_DIRNAME=$(cd "$(dirname "$1")" && pwd) || { - echo "Failed to change to test directory: $(dirname "$1")" >&2 - exit 1 -} +TEST_DIRNAME=$(cd "$(dirname "$1")" && pwd) TEST_FILENAME=$(basename "$1") shift TEST_EXECUTABLE="$TEST_DIRNAME/$TEST_FILENAME" @@ -50,10 +46,7 @@ TEST_NAME=$(echo "$TEST_FILENAME" | sed -E -e 's/\..+$//') # Remove path and ext # We run each test in its own subdir to avoid core file related races. TEST_WORKDIR="$OUTPUT_ROOT/build/test-work/$TEST_NAME" mkdir -p "$TEST_WORKDIR" -pushd "$TEST_WORKDIR" >/dev/null || { - echo "Failed to change to test working directory: $TEST_WORKDIR" >&2 - exit 1 -} +pushd "$TEST_WORKDIR" >/dev/null rm -f ./* set -o pipefail @@ -243,10 +236,7 @@ fi print_coredumps -popd || { - echo "Failed to restore the previous working directory" >&2 - exit 1 -} +popd rm -Rf "$TEST_WORKDIR" exit "$STATUS"

This change cause the following errors.

AMD64 Conda C++ AVX2 UNKNOWN STEP 2026-08-24T11:10:33.1775143Z Total Test time (real) = 548.94 sec AMD64 Conda C++ AVX2 UNKNOWN STEP 2026-08-24T11:10:33.1775314Z AMD64 Conda C++ AVX2 UNKNOWN STEP 2026-08-24T11:10:33.1775405Z The following tests FAILED: AMD64 Conda C++ AVX2 UNKNOWN STEP 2026-08-24T11:10:33.1775759Z 2 - arrow-buffer-test (Failed) arrow-tests unittest AMD64 Conda C++ AVX2 UNKNOWN STEP 2026-08-24T11:10:33.1776240Z 3 - arrow-extension-type-test (Failed) arrow-tests unittest AMD64 Conda C++ AVX2 UNKNOWN STEP 2026-08-24T11:10:33.1776688Z 4 - arrow-misc-test (Failed) arrow-tests unittest AMD64 Conda C++ AVX2 UNKNOWN STEP 2026-08-24T11:10:33.1777139Z 5 - arrow-public-api-test (Failed) arrow-tests unittest AMD64 Conda C++ AVX2 UNKNOWN STEP 2026-08-24T11:10:33.1777585Z 6 - arrow-scalar-test (Failed) arrow-tests unittest AMD64 Conda C++ AVX2 UNKNOWN STEP 2026-08-24T11:10:33.1778010Z 7 - arrow-type-test (Failed) arrow-tests unittest AMD64 Conda C++ AVX2 UNKNOWN STEP 2026-08-24T11:10:33.1778430Z 8 - arrow-table-test (Failed) arrow-tests unittest AMD64 Conda C++ AVX2 UNKNOWN STEP 2026-08-24T11:10:33.1778858Z 9 - arrow-tensor-test (Failed) arrow-tests unittest AMD64 Conda C++ AVX2 UNKNOWN STEP 2026-08-24T11:10:33.1779308Z 10 - arrow-sparse-tensor-test (Failed) arrow-tests unittest AMD64 Conda C++ AVX2 UNKNOWN STEP 2026-08-24T11:10:33.1779745Z 11 - arrow-stl-test (Failed) arrow-tests unittest AMD64 Conda C++ AVX2 UNKNOWN STEP 2026-08-24T11:10:33.1780176Z 18 - arrow-generator-test (Failed) arrow-tests unittest

How about this?

diff --git a/cpp/build-support/run-test.sh b/cpp/build-support/run-test.sh index 20e225d8dd..b992a21226 100755 --- a/cpp/build-support/run-test.sh +++ b/cpp/build-support/run-test.sh @@ -92,8 +92,11 @@ function run_test() { # even when retries are successful. rm -f $XMLFILE - $TEST_EXECUTABLE "$@" > $LOGFILE.raw 2>&1 - STATUS=$? + if $TEST_EXECUTABLE "$@" > $LOGFILE.raw 2>&1; then + STATUS=0 + else + STATUS=1 + fi cat $LOGFILE.raw \ | ${PYTHON:-python} $ROOT/build-support/asan_symbolize.py \ | ${CXXFILT:-c++filt} \

Thank you for your comments. I'll try it. It looks like we need to make changes not only to this part, but also to other parts.

This reverts commit 45bba12.

This reverts commit 4ed6bae.

github-actions Bot added Component: C++ awaiting review Awaiting review labels Aug 21, 2026

hiroyuki-sato mentioned this pull request Aug 22, 2026

GH-50934: [C++] Fix shellcheck errors in cpp/build-support/run-test.sh #50950

Open

hiroyuki-sato marked this pull request as ready for review August 22, 2026 07:22

hiroyuki-sato requested a review from raulcd as a code owner August 22, 2026 07:22

hiroyuki-sato force-pushed the topic/shellcheck-cpp-run-test branch from da4e451 to 7b9e1a6 Compare August 23, 2026 08:30

kou changed the title ~~GH-50934: Fix shellcheck errors in cpp/build-support/run-test.sh~~ GH-50934: [C++][Dev] Fix shellcheck errors in cpp/build-support/run-test.sh Aug 24, 2026

kou reviewed Aug 24, 2026

View reviewed changes

github-actions Bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting review Awaiting review awaiting changes Awaiting changes labels Aug 24, 2026

hiroyuki-sato marked this pull request as draft August 24, 2026 05:31

hiroyuki-sato added 6 commits August 24, 2026 19:57

apacheGH-50934: Fix shellcheck errors in cpp/build-support/run-test.sh

35aeedc

Revert .pre-commit-config.yaml

52776ea

Fix wrong remove variable

c6dc479

Quote variable expansions

65145f4

Use set -euo pipmail

4ed6bae

Use set -e

45bba12

hiroyuki-sato force-pushed the topic/shellcheck-cpp-run-test branch from bc92179 to 45bba12 Compare August 24, 2026 10:57

hiroyuki-sato added 2 commits August 24, 2026 21:15

Revert "Use set -e"

ef48615

This reverts commit 45bba12.

Revert "Use set -euo pipmail"

c24e25d

This reverts commit 4ed6bae.

github-actions Bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Aug 24, 2026

Navigation Menu

Uh oh!

GH-50934: [C++][Dev] Fix shellcheck errors in cpp/build-support/run-test.sh - #50935

GH-50934: [C++][Dev] Fix shellcheck errors in cpp/build-support/run-test.sh#50935
hiroyuki-sato wants to merge 8 commits into
apache:mainfrom
hiroyuki-sato:topic/shellcheck-cpp-run-test

hiroyuki-sato commented Aug 21, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 21, 2026

Uh oh!

hiroyuki-sato commented Aug 23, 2026

Uh oh!

hiroyuki-sato commented Aug 23, 2026

Uh oh!

tadeja commented Aug 23, 2026

Uh oh!

hiroyuki-sato commented Aug 24, 2026

Uh oh!

kou Aug 24, 2026

Uh oh!

hiroyuki-sato Aug 24, 2026

Uh oh!

kou Aug 24, 2026

Uh oh!

hiroyuki-sato Aug 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

hiroyuki-sato commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

github-actions Bot commented Aug 21, 2026

Uh oh!

hiroyuki-sato commented Aug 23, 2026

Uh oh!

hiroyuki-sato commented Aug 23, 2026

Uh oh!

tadeja commented Aug 23, 2026

Uh oh!

hiroyuki-sato commented Aug 24, 2026

Uh oh!

kou Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

hiroyuki-sato Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

kou Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

hiroyuki-sato Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hiroyuki-sato commented Aug 21, 2026 •

edited

Loading