From: SeongJae Park sjpark@amazon.de
When running a test program, 'run_one()' checks if the program has the execution permission and fails if it doesn't. However, it's easy to mistakenly missing the permission, as some common tools like 'diff' don't support the permission change well[1]. Compared to that, making mistakes in the test program's path would only rare, as those are explicitly listed in 'TEST_PROGS'. Therefore, it might make more sense to resolve the situation on our own and run the program.
For the reason, this commit makes the test program runner function to still print the warning message but run the program after giving the execution permission in the case. To make nothing corrupted, it also restores the permission after running it.
[1] https://lore.kernel.org/mm-commits/YRJisBs9AunccCD4@kroah.com/
Suggested-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: SeongJae Park sjpark@amazon.de --- tools/testing/selftests/kselftest/runner.sh | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh index cc9c846585f0..2eb31e945709 100644 --- a/tools/testing/selftests/kselftest/runner.sh +++ b/tools/testing/selftests/kselftest/runner.sh @@ -65,15 +65,16 @@ run_one()
TEST_HDR_MSG="selftests: $DIR: $BASENAME_TEST" echo "# $TEST_HDR_MSG" - if [ ! -x "$TEST" ]; then - echo -n "# Warning: file $TEST is " - if [ ! -e "$TEST" ]; then - echo "missing!" - else - echo "not executable, correct this." - fi + if [ ! -e "$TEST" ]; then + echo "# Warning: file $TEST is missing!" echo "not ok $test_num $TEST_HDR_MSG" else + permission_added="false" + if [ ! -x "$TEST" ]; then + echo "# Warning: file $TEST is not executable" + chmod u+x "$TEST" + permission_added="true" + fi cd `dirname $TEST` > /dev/null ((((( tap_timeout ./$BASENAME_TEST 2>&1; echo $? >&3) | tap_prefix >&4) 3>&1) | @@ -88,6 +89,9 @@ run_one() else echo "not ok $test_num $TEST_HDR_MSG # exit=$rc" fi) + if [ "$permission_added" = "true" ]; then + chmod u-x "$TEST" + fi cd - >/dev/null fi }
On Tue, Aug 10, 2021 at 02:04:59PM +0000, SeongJae Park wrote:
From: SeongJae Park sjpark@amazon.de
When running a test program, 'run_one()' checks if the program has the execution permission and fails if it doesn't. However, it's easy to mistakenly missing the permission, as some common tools like 'diff' don't support the permission change well[1]. Compared to that, making mistakes in the test program's path would only rare, as those are explicitly listed in 'TEST_PROGS'. Therefore, it might make more sense to resolve the situation on our own and run the program.
For the reason, this commit makes the test program runner function to still print the warning message but run the program after giving the execution permission in the case. To make nothing corrupted, it also restores the permission after running it.
[1] https://lore.kernel.org/mm-commits/YRJisBs9AunccCD4@kroah.com/
Suggested-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: SeongJae Park sjpark@amazon.de
tools/testing/selftests/kselftest/runner.sh | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh index cc9c846585f0..2eb31e945709 100644 --- a/tools/testing/selftests/kselftest/runner.sh +++ b/tools/testing/selftests/kselftest/runner.sh @@ -65,15 +65,16 @@ run_one() TEST_HDR_MSG="selftests: $DIR: $BASENAME_TEST" echo "# $TEST_HDR_MSG"
- if [ ! -x "$TEST" ]; then
echo -n "# Warning: file $TEST is "if [ ! -e "$TEST" ]; thenecho "missing!"elseecho "not executable, correct this."fi
- if [ ! -e "$TEST" ]; then
echo "# Warning: file $TEST is missing!"permission_added="false"if [ ! -x "$TEST" ]; thenecho "# Warning: file $TEST is not executable"chmod u+x "$TEST"permission_added="true"
No, why would you change the permission of a test? What happens if this is on a read-only filesystem? You should not be modifying it as it will end up causing changes when not needed.
thanks,
greg k-h
From: SeongJae Park sjpark@amazon.de
On Tue, 10 Aug 2021 17:07:28 +0200 Greg KH gregkh@linuxfoundation.org wrote:
On Tue, Aug 10, 2021 at 02:04:59PM +0000, SeongJae Park wrote:
From: SeongJae Park sjpark@amazon.de
When running a test program, 'run_one()' checks if the program has the execution permission and fails if it doesn't. However, it's easy to mistakenly missing the permission, as some common tools like 'diff' don't support the permission change well[1]. Compared to that, making mistakes in the test program's path would only rare, as those are explicitly listed in 'TEST_PROGS'. Therefore, it might make more sense to resolve the situation on our own and run the program.
For the reason, this commit makes the test program runner function to still print the warning message but run the program after giving the execution permission in the case. To make nothing corrupted, it also restores the permission after running it.
[1] https://lore.kernel.org/mm-commits/YRJisBs9AunccCD4@kroah.com/
Suggested-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: SeongJae Park sjpark@amazon.de
tools/testing/selftests/kselftest/runner.sh | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh index cc9c846585f0..2eb31e945709 100644 --- a/tools/testing/selftests/kselftest/runner.sh +++ b/tools/testing/selftests/kselftest/runner.sh @@ -65,15 +65,16 @@ run_one() TEST_HDR_MSG="selftests: $DIR: $BASENAME_TEST" echo "# $TEST_HDR_MSG"
- if [ ! -x "$TEST" ]; then
echo -n "# Warning: file $TEST is "if [ ! -e "$TEST" ]; thenecho "missing!"elseecho "not executable, correct this."fi
- if [ ! -e "$TEST" ]; then
echo "# Warning: file $TEST is missing!"permission_added="false"if [ ! -x "$TEST" ]; thenecho "# Warning: file $TEST is not executable"chmod u+x "$TEST"permission_added="true"No, why would you change the permission of a test? What happens if this is on a read-only filesystem? You should not be modifying it as it will end up causing changes when not needed.
Agreed. I will parse the shebang line and use the interpreter explicitly in the next spin.
Thanks, SeongJae Park
thanks,
greg k-h
linux-kselftest-mirror@lists.linaro.org
-
Greg KH -
SeongJae Park