Skip to content

Commit 9d9df70

Browse files
committed
[MERGE #6128 @koalaman] Allow SAFE_RUN to correctly run and check results
Merge pull request #6128 from koalaman:master Hi, I came across [this shell function](https://github.com/microsoft/ChakraCore/blob/68d19b73e4c796540d2d44cc17ac2320c7576cd0/test/native-tests/test_native.sh#L30) in `test/native-tests/test_native.sh`: ``` SAFE_RUN() { local SF_RETURN_VALUE=$($1 2>&1) if [[ $? != 0 ]]; then >&2 echo $SF_RETURN_VALUE exit 1 fi } ``` invoked as e.g. ``` SAFE_RUN `cd $TEST_PATH; ${CH_DIR} Platform.js > Makefile` ``` There are multiple issues that prevent it from living up to its name: 1. The code to run is executed by the backticks before the function ever starts: ``` $ test_run() { true; } $ test_run `ls /doesnotexist` ls: cannot access '/doesnotexist': No such file or directory ``` 2. If the invocation is changed to pass the command directly, the invocation fails for metacharacters like `;` and `>`. Here they are both treated as literal text: ``` $ test_run() { local var=$($1 >&2); echo "$var"; } $ test_run "echo Hello; ls > filelist" Hello; ls > filelist ``` 3. If passed a valid invocation, `local` [masks all exit codes](https://github.com/koalaman/shellcheck/wiki/SC2155) and returns success, even when the command fails: ``` $ test_run() { local foo=$($1); echo "$1 returned $?"; } $ test_run true true returned 0 $ test_run false false returned 0 ``` The suggested commits fixes all these issues. I was unable to verify the fix locally as the tests appeared to hang when run overnight with and without these fixes. I'm not quite sure how they're used, but there is some possibility that this commit uncovers failures that were previously hidden.
2 parents c0ae3d3 + c2fc081 commit 9d9df70

File tree

1 file changed

+8
-7
lines changed

1 file changed

+8
-7
lines changed

test/native-tests/test_native.sh

+8-7
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,11 @@ FIND_CLANG() {
2828
}
2929

3030
SAFE_RUN() {
31-
local SF_RETURN_VALUE=$($1 2>&1)
31+
local SF_OUTPUT
32+
SF_OUTPUT=$(eval "( $1 )" 2>&1)
3233

3334
if [[ $? != 0 ]]; then
34-
>&2 echo $SF_RETURN_VALUE
35+
>&2 echo "$SF_OUTPUT"
3536
exit 1
3637
fi
3738
}
@@ -57,24 +58,24 @@ fi
5758
RUN () {
5859
TEST_PATH=$1
5960
echo "Testing $TEST_PATH"
60-
SAFE_RUN `cd $TEST_PATH; ${CH_DIR} Platform.js > Makefile`
61+
SAFE_RUN "cd $TEST_PATH && ${CH_DIR} Platform.js > Makefile"
6162
RES=$(cd $TEST_PATH; cat Makefile)
6263

6364
if [[ $RES =~ "# IGNORE_THIS_TEST" ]]; then
6465
echo "Ignoring $TEST_PATH"
6566
else
66-
SAFE_RUN `cd $TEST_PATH; make CC=${CC} CXX=${CXX}`
67+
SAFE_RUN "cd $TEST_PATH && make CC=${CC} CXX=${CXX}"
6768
RES=$(cd $TEST_PATH; ./sample.o)
6869
TEST "SUCCESS"
69-
SAFE_RUN `cd $TEST_PATH; rm -rf ./sample.o`
70+
SAFE_RUN "cd $TEST_PATH && rm -rf ./sample.o"
7071
fi
7172
}
7273

7374
RUN_CMD () {
7475
TEST_PATH=$1
7576
CMD=$2
7677
echo "Testing $TEST_PATH"
77-
SAFE_RUN `cd $TEST_PATH; $CMD`
78+
SAFE_RUN "cd $TEST_PATH && $CMD"
7879
}
7980

8081
# static lib tests
@@ -100,4 +101,4 @@ RUN "test-shared-basic"
100101
# test python
101102
RUN_CMD "test-python" "python helloWorld.py ${BUILD_TYPE}"
102103

103-
SAFE_RUN `rm -rf Makefile`
104+
SAFE_RUN "rm -rf Makefile"

0 commit comments

Comments
 (0)