When ion test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/android/ion/ion_test.sh | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/android/ion/ion_test.sh b/tools/testing/selftests/android/ion/ion_test.sh index a1aff506f5e6..69e676cfc94e 100755 --- a/tools/testing/selftests/android/ion/ion_test.sh +++ b/tools/testing/selftests/android/ion/ion_test.sh @@ -4,6 +4,9 @@ heapsize=4096 TCID="ion_test.sh" errcode=0
+# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + run_test() { heaptype=$1 @@ -25,7 +28,7 @@ check_root() uid=$(id -u) if [ $uid -ne 0 ]; then echo $TCID: must be run as root >&2 - exit 0 + exit $ksft_skip fi }
@@ -35,7 +38,7 @@ check_device() if [ ! -e $DEVICE ]; then echo $TCID: No $DEVICE device found >&2 echo $TCID: May be CONFIG_ION is not set >&2 - exit 0 + exit $ksft_skip fi }
When step_after_suspend_test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Change it to use ksft_exit_skip() when a non-root user runs the test and add an explicit check for root and a clear message, instead of failing the test when /sys/power/state file open fails.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/breakpoints/step_after_suspend_test.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/breakpoints/step_after_suspend_test.c b/tools/testing/selftests/breakpoints/step_after_suspend_test.c index 3fece06e9f64..f82dcc1f8841 100644 --- a/tools/testing/selftests/breakpoints/step_after_suspend_test.c +++ b/tools/testing/selftests/breakpoints/step_after_suspend_test.c @@ -143,10 +143,14 @@ void suspend(void) int err; struct itimerspec spec = {};
+ if (getuid() != 0) + ksft_exit_skip("Please run the test as root - Exiting.\n"); + power_state_fd = open("/sys/power/state", O_RDWR); if (power_state_fd < 0) ksft_exit_fail_msg( - "open("/sys/power/state") failed (is this test running as root?)\n"); + "open("/sys/power/state") failed %s)\n", + strerror(errno));
timerfd = timerfd_create(CLOCK_BOOTTIME_ALARM, 0); if (timerfd < 0)
When cpu-on-off-test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh b/tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh index f3a8933c1275..bab13dd025a6 100755 --- a/tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh +++ b/tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh @@ -2,6 +2,8 @@ # SPDX-License-Identifier: GPL-2.0
SYSFS= +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4
prerequisite() { @@ -9,7 +11,7 @@ prerequisite()
if [ $UID != 0 ]; then echo $msg must be run as root >&2 - exit 0 + exit $ksft_skip fi
taskset -p 01 $$ @@ -18,12 +20,12 @@ prerequisite()
if [ ! -d "$SYSFS" ]; then echo $msg sysfs is not mounted >&2 - exit 0 + exit $ksft_skip fi
if ! ls $SYSFS/devices/system/cpu/cpu* > /dev/null 2>&1; then echo $msg cpu hotplug is not supported >&2 - exit 0 + exit $ksft_skip fi
echo "CPU online/offline summary:" @@ -32,7 +34,7 @@ prerequisite()
if [[ "$online_cpus" = "$online_max" ]]; then echo "$msg: since there is only one cpu: $online_cpus" - exit 0 + exit $ksft_skip fi
echo -e "\t Cpus in online state: $online_cpus" @@ -237,12 +239,12 @@ prerequisite_extra()
if [ ! -d "$DEBUGFS" ]; then echo $msg debugfs is not mounted >&2 - exit 0 + exit $ksft_skip fi
if [ ! -d $NOTIFIER_ERR_INJECT_DIR ]; then echo $msg cpu-notifier-error-inject module is not available >&2 - exit 0 + exit $ksft_skip fi }
When cpufreq test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/cpufreq/main.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/cpufreq/main.sh b/tools/testing/selftests/cpufreq/main.sh index d83922de9d89..31f8c9a76c5f 100755 --- a/tools/testing/selftests/cpufreq/main.sh +++ b/tools/testing/selftests/cpufreq/main.sh @@ -13,6 +13,9 @@ SYSFS= CPUROOT= CPUFREQROOT=
+# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + helpme() { printf "Usage: $0 [-h] [-todg args] @@ -38,7 +41,7 @@ prerequisite()
if [ $UID != 0 ]; then echo $msg must be run as root >&2 - exit 2 + exit $ksft_skip fi
taskset -p 01 $$
On 04-05-18, 19:13, Shuah Khan (Samsung OSG) wrote:
When cpufreq test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org
tools/testing/selftests/cpufreq/main.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/cpufreq/main.sh b/tools/testing/selftests/cpufreq/main.sh index d83922de9d89..31f8c9a76c5f 100755 --- a/tools/testing/selftests/cpufreq/main.sh +++ b/tools/testing/selftests/cpufreq/main.sh @@ -13,6 +13,9 @@ SYSFS= CPUROOT= CPUFREQROOT= +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4
helpme() { printf "Usage: $0 [-h] [-todg args] @@ -38,7 +41,7 @@ prerequisite() if [ $UID != 0 ]; then echo $msg must be run as root >&2
exit 2
fiexit $ksft_skip
taskset -p 01 $$
Acked-by: Viresh Kumar viresh.kumar@linaro.org
When efivarfs test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/efivarfs/efivarfs.sh | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/efivarfs/efivarfs.sh b/tools/testing/selftests/efivarfs/efivarfs.sh index c6d5790575ae..a47029a799d2 100755 --- a/tools/testing/selftests/efivarfs/efivarfs.sh +++ b/tools/testing/selftests/efivarfs/efivarfs.sh @@ -4,18 +4,21 @@ efivarfs_mount=/sys/firmware/efi/efivars test_guid=210be57c-9849-4fc7-a635-e6382d1aec27
+# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + check_prereqs() { local msg="skip all tests:"
if [ $UID != 0 ]; then echo $msg must be run as root >&2 - exit 0 + exit $ksft_skip fi
if ! grep -q "^\S+ $efivarfs_mount efivarfs" /proc/mounts; then echo $msg efivarfs is not mounted on $efivarfs_mount >&2 - exit 0 + exit $ksft_skip fi }
When execveat test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Change it to use ksft_exit_skip() when kernel doesn't support execveat.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/exec/execveat.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/exec/execveat.c b/tools/testing/selftests/exec/execveat.c index 67cd4597db2b..47cbf54d0801 100644 --- a/tools/testing/selftests/exec/execveat.c +++ b/tools/testing/selftests/exec/execveat.c @@ -20,6 +20,8 @@ #include <string.h> #include <unistd.h>
+#include "../kselftest.h" + static char longpath[2 * PATH_MAX] = ""; static char *envp[] = { "IN_TEST=yes", NULL, NULL }; static char *argv[] = { "execveat", "99", NULL }; @@ -249,8 +251,8 @@ static int run_tests(void) errno = 0; execveat_(-1, NULL, NULL, NULL, 0); if (errno == ENOSYS) { - printf("[FAIL] ENOSYS calling execveat - no kernel support?\n"); - return 1; + ksft_exit_skip( + "ENOSYS calling execveat - no kernel support?\n"); }
/* Change file position to confirm it doesn't affect anything */
On 05/04/2018 06:13 PM, Shuah Khan (Samsung OSG) wrote:
When execveat test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Change it to use ksft_exit_skip() when kernel doesn't support execveat.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org
tools/testing/selftests/exec/execveat.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/exec/execveat.c b/tools/testing/selftests/exec/execveat.c index 67cd4597db2b..47cbf54d0801 100644 --- a/tools/testing/selftests/exec/execveat.c +++ b/tools/testing/selftests/exec/execveat.c @@ -20,6 +20,8 @@ #include <string.h> #include <unistd.h> +#include "../kselftest.h"
- static char longpath[2 * PATH_MAX] = ""; static char *envp[] = { "IN_TEST=yes", NULL, NULL }; static char *argv[] = { "execveat", "99", NULL };
@@ -249,8 +251,8 @@ static int run_tests(void) errno = 0; execveat_(-1, NULL, NULL, NULL, 0); if (errno == ENOSYS) {
printf("[FAIL] ENOSYS calling execveat - no kernel support?\n");
return 1;
ksft_exit_skip(
}"ENOSYS calling execveat - no kernel support?\n");
/* Change file position to confirm it doesn't affect anything */
LGTM.
thanks, Steve -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
When devpts_pts test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
In another case, it returns pass for a skipped test reporting a false postive.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Change it to use ksft_exit_skip() when test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/filesystems/devpts_pts.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/filesystems/devpts_pts.c b/tools/testing/selftests/filesystems/devpts_pts.c index b9055e974289..0f2d9f427944 100644 --- a/tools/testing/selftests/filesystems/devpts_pts.c +++ b/tools/testing/selftests/filesystems/devpts_pts.c @@ -11,6 +11,7 @@ #include <sys/ioctl.h> #include <sys/mount.h> #include <sys/wait.h> +#include "../kselftest.h"
static bool terminal_dup2(int duplicate, int original) { @@ -125,10 +126,12 @@ static int do_tiocgptpeer(char *ptmx, char *expected_procfd_contents) if (errno == EINVAL) { fprintf(stderr, "TIOCGPTPEER is not supported. " "Skipping test.\n"); - fret = EXIT_SUCCESS; + fret = KSFT_SKIP; + } else { + fprintf(stderr, + "Failed to perform TIOCGPTPEER ioctl\n"); + fret = EXIT_FAILURE; } - - fprintf(stderr, "Failed to perform TIOCGPTPEER ioctl\n"); goto do_cleanup; }
@@ -281,7 +284,7 @@ int main(int argc, char *argv[]) if (!isatty(STDIN_FILENO)) { fprintf(stderr, "Standard input file desciptor is not attached " "to a terminal. Skipping test\n"); - exit(EXIT_FAILURE); + exit(KSFT_SKIP); }
ret = unshare(CLONE_NEWNS);
On Fri, May 04, 2018 at 07:13:11PM -0600, Shuah Khan (Samsung OSG) wrote:
When devpts_pts test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
In another case, it returns pass for a skipped test reporting a false postive.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Change it to use ksft_exit_skip() when test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org
Acked-by: Christian Brauner christian.brauner@ubuntu.com
Thanks, Shuah! Christian
tools/testing/selftests/filesystems/devpts_pts.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/filesystems/devpts_pts.c b/tools/testing/selftests/filesystems/devpts_pts.c index b9055e974289..0f2d9f427944 100644 --- a/tools/testing/selftests/filesystems/devpts_pts.c +++ b/tools/testing/selftests/filesystems/devpts_pts.c @@ -11,6 +11,7 @@ #include <sys/ioctl.h> #include <sys/mount.h> #include <sys/wait.h> +#include "../kselftest.h" static bool terminal_dup2(int duplicate, int original) { @@ -125,10 +126,12 @@ static int do_tiocgptpeer(char *ptmx, char *expected_procfd_contents) if (errno == EINVAL) { fprintf(stderr, "TIOCGPTPEER is not supported. " "Skipping test.\n");
fret = EXIT_SUCCESS;
fret = KSFT_SKIP;
} else {
fprintf(stderr,
"Failed to perform TIOCGPTPEER ioctl\n");
}fret = EXIT_FAILURE;
goto do_cleanup; }fprintf(stderr, "Failed to perform TIOCGPTPEER ioctl\n");
@@ -281,7 +284,7 @@ int main(int argc, char *argv[]) if (!isatty(STDIN_FILENO)) { fprintf(stderr, "Standard input file desciptor is not attached " "to a terminal. Skipping test\n");
exit(EXIT_FAILURE);
}exit(KSFT_SKIP);
ret = unshare(CLONE_NEWNS); -- 2.14.1
-- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
When firmware test(s) get skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/firmware/fw_fallback.sh | 4 ++-- tools/testing/selftests/firmware/fw_filesystem.sh | 4 +++- tools/testing/selftests/firmware/fw_lib.sh | 7 +++++-- 3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/firmware/fw_fallback.sh b/tools/testing/selftests/firmware/fw_fallback.sh index 8e2e34a2ca69..70d18be46af5 100755 --- a/tools/testing/selftests/firmware/fw_fallback.sh +++ b/tools/testing/selftests/firmware/fw_fallback.sh @@ -74,7 +74,7 @@ load_fw_custom() { if [ ! -e "$DIR"/trigger_custom_fallback ]; then echo "$0: custom fallback trigger not present, ignoring test" >&2 - return 1 + exit $ksft_skip fi
local name="$1" @@ -107,7 +107,7 @@ load_fw_custom_cancel() { if [ ! -e "$DIR"/trigger_custom_fallback ]; then echo "$0: canceling custom fallback trigger not present, ignoring test" >&2 - return 1 + exit $ksft_skip fi
local name="$1" diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh index 6452d2129cd9..a4320c4b44dc 100755 --- a/tools/testing/selftests/firmware/fw_filesystem.sh +++ b/tools/testing/selftests/firmware/fw_filesystem.sh @@ -30,6 +30,7 @@ fi
if [ ! -e "$DIR"/trigger_async_request ]; then echo "$0: empty filename: async trigger not present, ignoring test" >&2 + exit $ksft_skip else if printf '\000' >"$DIR"/trigger_async_request 2> /dev/null; then echo "$0: empty filename should not succeed (async)" >&2 @@ -69,6 +70,7 @@ fi # Try the asynchronous version too if [ ! -e "$DIR"/trigger_async_request ]; then echo "$0: firmware loading: async trigger not present, ignoring test" >&2 + exit $ksft_skip else if ! echo -n "$NAME" >"$DIR"/trigger_async_request ; then echo "$0: could not trigger async request" >&2 @@ -89,7 +91,7 @@ test_config_present() { if [ ! -f $DIR/reset ]; then echo "Configuration triggers not present, ignoring test" - exit 0 + exit $ksft_skip fi }
diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh index 962d7f4ac627..6c5f1b2ffb74 100755 --- a/tools/testing/selftests/firmware/fw_lib.sh +++ b/tools/testing/selftests/firmware/fw_lib.sh @@ -9,11 +9,14 @@ DIR=/sys/devices/virtual/misc/test_firmware PROC_CONFIG="/proc/config.gz" TEST_DIR=$(dirname $0)
+# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + print_reqs_exit() { echo "You must have the following enabled in your kernel:" >&2 cat $TEST_DIR/config >&2 - exit 1 + exit $ksft_skip }
test_modprobe() @@ -88,7 +91,7 @@ verify_reqs() if [ "$TEST_REQS_FW_SYSFS_FALLBACK" = "yes" ]; then if [ ! "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then echo "usermode helper disabled so ignoring test" - exit 0 + exit $ksft_skip fi fi }
On Fri, May 04, 2018 at 07:13:12PM -0600, Shuah Khan (Samsung OSG) wrote:
When firmware test(s) get skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org
Reviewed-by: Luis R. Rodriguez mcgrof@kernel.org
Luis
tools/testing/selftests/firmware/fw_fallback.sh | 4 ++-- tools/testing/selftests/firmware/fw_filesystem.sh | 4 +++- tools/testing/selftests/firmware/fw_lib.sh | 7 +++++-- 3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/firmware/fw_fallback.sh b/tools/testing/selftests/firmware/fw_fallback.sh index 8e2e34a2ca69..70d18be46af5 100755 --- a/tools/testing/selftests/firmware/fw_fallback.sh +++ b/tools/testing/selftests/firmware/fw_fallback.sh @@ -74,7 +74,7 @@ load_fw_custom() { if [ ! -e "$DIR"/trigger_custom_fallback ]; then echo "$0: custom fallback trigger not present, ignoring test" >&2
return 1
fiexit $ksft_skip
local name="$1" @@ -107,7 +107,7 @@ load_fw_custom_cancel() { if [ ! -e "$DIR"/trigger_custom_fallback ]; then echo "$0: canceling custom fallback trigger not present, ignoring test" >&2
return 1
fiexit $ksft_skip
local name="$1" diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh index 6452d2129cd9..a4320c4b44dc 100755 --- a/tools/testing/selftests/firmware/fw_filesystem.sh +++ b/tools/testing/selftests/firmware/fw_filesystem.sh @@ -30,6 +30,7 @@ fi if [ ! -e "$DIR"/trigger_async_request ]; then echo "$0: empty filename: async trigger not present, ignoring test" >&2
- exit $ksft_skip
else if printf '\000' >"$DIR"/trigger_async_request 2> /dev/null; then echo "$0: empty filename should not succeed (async)" >&2 @@ -69,6 +70,7 @@ fi # Try the asynchronous version too if [ ! -e "$DIR"/trigger_async_request ]; then echo "$0: firmware loading: async trigger not present, ignoring test" >&2
- exit $ksft_skip
else if ! echo -n "$NAME" >"$DIR"/trigger_async_request ; then echo "$0: could not trigger async request" >&2 @@ -89,7 +91,7 @@ test_config_present() { if [ ! -f $DIR/reset ]; then echo "Configuration triggers not present, ignoring test"
exit 0
fiexit $ksft_skip
} diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh index 962d7f4ac627..6c5f1b2ffb74 100755 --- a/tools/testing/selftests/firmware/fw_lib.sh +++ b/tools/testing/selftests/firmware/fw_lib.sh @@ -9,11 +9,14 @@ DIR=/sys/devices/virtual/misc/test_firmware PROC_CONFIG="/proc/config.gz" TEST_DIR=$(dirname $0) +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4
print_reqs_exit() { echo "You must have the following enabled in your kernel:" >&2 cat $TEST_DIR/config >&2
- exit 1
- exit $ksft_skip
} test_modprobe() @@ -88,7 +91,7 @@ verify_reqs() if [ "$TEST_REQS_FW_SYSFS_FALLBACK" = "yes" ]; then if [ ! "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then echo "usermode helper disabled so ignoring test"
exit 0
fi fiexit $ksft_skip
}
2.14.1
When ftrace test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/ftrace/ftracetest | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest index f9a9d424c980..b731c8cdcffb 100755 --- a/tools/testing/selftests/ftrace/ftracetest +++ b/tools/testing/selftests/ftrace/ftracetest @@ -23,6 +23,9 @@ echo " If <dir> is -, all logs output in console only" exit $1 }
+# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + errexit() { # message echo "Error: $1" 1>&2 exit 1 @@ -30,7 +33,8 @@ errexit() { # message
# Ensuring user privilege if [ `id -u` -ne 0 ]; then - errexit "this must be run by root user" + echo "Skipping: test must be run by root user" + exit $ksft_skip fi
# Utilities @@ -249,7 +253,7 @@ trap 'SIG_RESULT=$UNTESTED' $SIG_UNTESTED SIG_UNSUPPORTED=$((SIG_BASE + UNSUPPORTED)) exit_unsupported () { kill -s $SIG_UNSUPPORTED $SIG_PID - exit 0 + exit $ksft_skip } trap 'SIG_RESULT=$UNSUPPORTED' $SIG_UNSUPPORTED
On Fri, 4 May 2018 19:13:13 -0600 "Shuah Khan (Samsung OSG)" shuah@kernel.org wrote:
When ftrace test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
I'm fine with this change, but I believe Masami and perhaps others have scripts that expect zero return.
If anything, we probably need to have a way to override the ksft_skip via a command line argument.
Masami?
-- Steve
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org
tools/testing/selftests/ftrace/ftracetest | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest index f9a9d424c980..b731c8cdcffb 100755 --- a/tools/testing/selftests/ftrace/ftracetest +++ b/tools/testing/selftests/ftrace/ftracetest @@ -23,6 +23,9 @@ echo " If <dir> is -, all logs output in console only" exit $1 } +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4
errexit() { # message echo "Error: $1" 1>&2 exit 1 @@ -30,7 +33,8 @@ errexit() { # message # Ensuring user privilege if [ `id -u` -ne 0 ]; then
- errexit "this must be run by root user"
- echo "Skipping: test must be run by root user"
- exit $ksft_skip
fi # Utilities @@ -249,7 +253,7 @@ trap 'SIG_RESULT=$UNTESTED' $SIG_UNTESTED SIG_UNSUPPORTED=$((SIG_BASE + UNSUPPORTED)) exit_unsupported () { kill -s $SIG_UNSUPPORTED $SIG_PID
- exit 0
- exit $ksft_skip
} trap 'SIG_RESULT=$UNSUPPORTED' $SIG_UNSUPPORTED
-- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 7 May 2018 11:17:21 -0400 Steven Rostedt rostedt@goodmis.org wrote:
On Fri, 4 May 2018 19:13:13 -0600 "Shuah Khan (Samsung OSG)" shuah@kernel.org wrote:
When ftrace test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
I'm fine with this change, but I believe Masami and perhaps others have scripts that expect zero return.
Oh, I'm happy with updating my script to follow the kselftest framework spec, since that is the standard.
Shuah, is there any document which describe the return code for kselftest framework? (And I think it should report some FAIL/PASS counts to kselftest
However, this patch seems not working as you expected. If kselftest framework would like to get the SKIP code from ftrace script, you need to update TOTAL_RESULT before leave.
If anything, we probably need to have a way to override the ksft_skip via a command line argument.
Masami?
Agreed. Maybe we can pass the option via an environment variable.
Thanks,
-- Steve
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org
tools/testing/selftests/ftrace/ftracetest | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest index f9a9d424c980..b731c8cdcffb 100755 --- a/tools/testing/selftests/ftrace/ftracetest +++ b/tools/testing/selftests/ftrace/ftracetest @@ -23,6 +23,9 @@ echo " If <dir> is -, all logs output in console only" exit $1 } +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4
errexit() { # message echo "Error: $1" 1>&2 exit 1 @@ -30,7 +33,8 @@ errexit() { # message # Ensuring user privilege if [ `id -u` -ne 0 ]; then
- errexit "this must be run by root user"
- echo "Skipping: test must be run by root user"
- exit $ksft_skip
fi # Utilities @@ -249,7 +253,7 @@ trap 'SIG_RESULT=$UNTESTED' $SIG_UNTESTED SIG_UNSUPPORTED=$((SIG_BASE + UNSUPPORTED)) exit_unsupported () { kill -s $SIG_UNSUPPORTED $SIG_PID
- exit 0
- exit $ksft_skip
} trap 'SIG_RESULT=$UNSUPPORTED' $SIG_UNSUPPORTED
On Fri, 4 May 2018 19:13:13 -0600 "Shuah Khan (Samsung OSG)" shuah@kernel.org wrote:
When ftrace test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org
tools/testing/selftests/ftrace/ftracetest | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest index f9a9d424c980..b731c8cdcffb 100755 --- a/tools/testing/selftests/ftrace/ftracetest +++ b/tools/testing/selftests/ftrace/ftracetest @@ -23,6 +23,9 @@ echo " If <dir> is -, all logs output in console only" exit $1 } +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4
errexit() { # message echo "Error: $1" 1>&2 exit 1 @@ -30,7 +33,8 @@ errexit() { # message # Ensuring user privilege if [ `id -u` -ne 0 ]; then
- errexit "this must be run by root user"
- echo "Skipping: test must be run by root user"
- exit $ksft_skip
fi # Utilities @@ -249,7 +253,7 @@ trap 'SIG_RESULT=$UNTESTED' $SIG_UNTESTED SIG_UNSUPPORTED=$((SIG_BASE + UNSUPPORTED)) exit_unsupported () { kill -s $SIG_UNSUPPORTED $SIG_PID
- exit 0
- exit $ksft_skip
This should return 0. If you want to change the result code, you have to change the last part as below. (Note that we need a switch option of return code, so that ftracetest user can continue to use same way...)
diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest index f9a9d424c980..d6ce56a2a937 100755 --- a/tools/testing/selftests/ftrace/ftracetest +++ b/tools/testing/selftests/ftrace/ftracetest @@ -326,5 +326,15 @@ prlog "# of unsupported: " `echo $UNSUPPORTED_CASES | wc -w` prlog "# of xfailed: " `echo $XFAILED_CASES | wc -w` prlog "# of undefined(test bug): " `echo $UNDEFINED_CASES | wc -w`
-# if no error, return 0 -exit $TOTAL_RESULT +# following kselftest result code +if [ $UNSUPPORTED_CASES -ne 0 -o \ + $UNTESTED_CASES -ne 0 -o \ + $UNRESOLVED_CASES -ne 0]; then # There are skipped tests + exit 4 +elif [ $XAILED_CASES -ne 0 -o ]; then # XFAILs + exit 2 +elif [ $FAILED_CASES -ne 0 -o ]; then # FAILs + exit 1 +else # PASS + exit 0 +fi
Thanks,
Hi Masami,
On 05/07/2018 09:38 PM, Masami Hiramatsu wrote:
On Fri, 4 May 2018 19:13:13 -0600 "Shuah Khan (Samsung OSG)" shuah@kernel.org wrote:
When ftrace test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org
tools/testing/selftests/ftrace/ftracetest | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest index f9a9d424c980..b731c8cdcffb 100755 --- a/tools/testing/selftests/ftrace/ftracetest +++ b/tools/testing/selftests/ftrace/ftracetest @@ -23,6 +23,9 @@ echo " If <dir> is -, all logs output in console only" exit $1 } +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4
errexit() { # message echo "Error: $1" 1>&2 exit 1 @@ -30,7 +33,8 @@ errexit() { # message # Ensuring user privilege if [ `id -u` -ne 0 ]; then
- errexit "this must be run by root user"
- echo "Skipping: test must be run by root user"
- exit $ksft_skip
fi # Utilities @@ -249,7 +253,7 @@ trap 'SIG_RESULT=$UNTESTED' $SIG_UNTESTED SIG_UNSUPPORTED=$((SIG_BASE + UNSUPPORTED)) exit_unsupported () { kill -s $SIG_UNSUPPORTED $SIG_PID
- exit 0
- exit $ksft_skip
This should return 0. If you want to change the result code, you have to change the last part as below. (Note that we need a switch option of return code, so that ftracetest user can continue to use same way...)
Doesn't this existing option take care of this:
--fail-unsupported Treat UNSUPPORTED as a failure"
Is there a need for another. This indicates that the default mode is UNSUPPORTED isn't a failure.
diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest index f9a9d424c980..d6ce56a2a937 100755 --- a/tools/testing/selftests/ftrace/ftracetest +++ b/tools/testing/selftests/ftrace/ftracetest @@ -326,5 +326,15 @@ prlog "# of unsupported: " `echo $UNSUPPORTED_CASES | wc -w` prlog "# of xfailed: " `echo $XFAILED_CASES | wc -w` prlog "# of undefined(test bug): " `echo $UNDEFINED_CASES | wc -w` -# if no error, return 0 -exit $TOTAL_RESULT +# following kselftest result code +if [ $UNSUPPORTED_CASES -ne 0 -o \
$UNTESTED_CASES -ne 0 -o \
$UNRESOLVED_CASES -ne 0]; then # There are skipped tests
- exit 4
+elif [ $XAILED_CASES -ne 0 -o ]; then # XFAILs
- exit 2
+elif [ $FAILED_CASES -ne 0 -o ]; then # FAILs
- exit 1
+else # PASS
- exit 0
+fi
Thanks for pointing out the right change to make.
-- Shuah
-- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 10 May 2018 14:18:16 -0600 Shuah Khan shuah@kernel.org wrote:
Hi Masami,
On 05/07/2018 09:38 PM, Masami Hiramatsu wrote:
On Fri, 4 May 2018 19:13:13 -0600 "Shuah Khan (Samsung OSG)" shuah@kernel.org wrote:
When ftrace test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org
tools/testing/selftests/ftrace/ftracetest | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest index f9a9d424c980..b731c8cdcffb 100755 --- a/tools/testing/selftests/ftrace/ftracetest +++ b/tools/testing/selftests/ftrace/ftracetest @@ -23,6 +23,9 @@ echo " If <dir> is -, all logs output in console only" exit $1 } +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4
errexit() { # message echo "Error: $1" 1>&2 exit 1 @@ -30,7 +33,8 @@ errexit() { # message # Ensuring user privilege if [ `id -u` -ne 0 ]; then
- errexit "this must be run by root user"
- echo "Skipping: test must be run by root user"
- exit $ksft_skip
fi # Utilities @@ -249,7 +253,7 @@ trap 'SIG_RESULT=$UNTESTED' $SIG_UNTESTED SIG_UNSUPPORTED=$((SIG_BASE + UNSUPPORTED)) exit_unsupported () { kill -s $SIG_UNSUPPORTED $SIG_PID
- exit 0
- exit $ksft_skip
This should return 0. If you want to change the result code, you have to change the last part as below. (Note that we need a switch option of return code, so that ftracetest user can continue to use same way...)
Doesn't this existing option take care of this:
--fail-unsupported Treat UNSUPPORTED as a failure"
No, that is an opposite option... it makes UNSUPPORTED result as a failure. What the option we need is to ignore UNSUPPORTED results because it is expected.
Is there a need for another. This indicates that the default mode is UNSUPPORTED isn't a failure.
Right, currently UNSUPPORTED/UNRESOLVED/UNTESTED are not failure.
diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest index f9a9d424c980..d6ce56a2a937 100755 --- a/tools/testing/selftests/ftrace/ftracetest +++ b/tools/testing/selftests/ftrace/ftracetest @@ -326,5 +326,15 @@ prlog "# of unsupported: " `echo $UNSUPPORTED_CASES | wc -w` prlog "# of xfailed: " `echo $XFAILED_CASES | wc -w` prlog "# of undefined(test bug): " `echo $UNDEFINED_CASES | wc -w` -# if no error, return 0 -exit $TOTAL_RESULT +# following kselftest result code +if [ $UNSUPPORTED_CASES -ne 0 -o \
$UNTESTED_CASES -ne 0 -o \
$UNRESOLVED_CASES -ne 0]; then # There are skipped tests
- exit 4
+elif [ $XAILED_CASES -ne 0 -o ]; then # XFAILs
Ah, I missed to remove the last "-o"
- exit 2
+elif [ $FAILED_CASES -ne 0 -o ]; then # FAILs
ditto.
Thanks,
- exit 1
+else # PASS
- exit 0
+fi
Thanks for pointing out the right change to make.
-- Shuah
When gpio test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/gpio/gpio-mockup.sh | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/gpio/gpio-mockup.sh b/tools/testing/selftests/gpio/gpio-mockup.sh index 183fb932edbd..7f35b9880485 100755 --- a/tools/testing/selftests/gpio/gpio-mockup.sh +++ b/tools/testing/selftests/gpio/gpio-mockup.sh @@ -2,10 +2,11 @@ # SPDX-License-Identifier: GPL-2.0
#exit status -#1: run as non-root user +#1: Internal error #2: sysfs/debugfs not mount #3: insert module fail when gpio-mockup is a module. -#4: other reason. +#4: Skip test including run as non-root user. +#5: other reason.
SYSFS= GPIO_SYSFS= @@ -15,6 +16,9 @@ GPIO_DEBUGFS= dev_type= module=
+# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + usage() { echo "Usage:" @@ -34,7 +38,7 @@ prerequisite() msg="skip all tests:" if [ $UID != 0 ]; then echo $msg must be run as root >&2 - exit 1 + exit $ksft_skip fi SYSFS=`mount -t sysfs | head -1 | awk '{ print $3 }'` if [ ! -d "$SYSFS" ]; then @@ -73,7 +77,7 @@ remove_module() die() { remove_module - exit 4 + exit 5 }
test_chips()
When intel_pstate test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/intel_pstate/run.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/intel_pstate/run.sh b/tools/testing/selftests/intel_pstate/run.sh index c670359becc6..6ded61670f6d 100755 --- a/tools/testing/selftests/intel_pstate/run.sh +++ b/tools/testing/selftests/intel_pstate/run.sh @@ -30,9 +30,12 @@
EVALUATE_ONLY=0
+# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + if ! uname -m | sed -e s/i.86/x86/ -e s/x86_64/x86/ | grep -q x86; then echo "$0 # Skipped: Test can only run on x86 architectures." - exit 0 + exit $ksft_skip fi
max_cpus=$(($(nproc)-1))
When ipc test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Change it to use ksft_exit_skip() when the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/ipc/msgque.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/ipc/msgque.c b/tools/testing/selftests/ipc/msgque.c index ee9382bdfadc..dac927e82336 100644 --- a/tools/testing/selftests/ipc/msgque.c +++ b/tools/testing/selftests/ipc/msgque.c @@ -196,10 +196,9 @@ int main(int argc, char **argv) int msg, pid, err; struct msgque_data msgque;
- if (getuid() != 0) { - printf("Please run the test as root - Exiting.\n"); - return ksft_exit_fail(); - } + if (getuid() != 0) + return ksft_exit_skip( + "Please run the test as root - Exiting.\n");
msgque.key = ftok(argv[0], 822155650); if (msgque.key == -1) {
When kmod test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run. It returns fail in some cases when test is skipped. Either way, it is incorrect and incosnistent reporting.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/kmod/kmod.sh | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/kmod/kmod.sh b/tools/testing/selftests/kmod/kmod.sh index 7956ea3be667..0a76314b4414 100755 --- a/tools/testing/selftests/kmod/kmod.sh +++ b/tools/testing/selftests/kmod/kmod.sh @@ -62,13 +62,16 @@ ALL_TESTS="$ALL_TESTS 0007:5:1" ALL_TESTS="$ALL_TESTS 0008:150:1" ALL_TESTS="$ALL_TESTS 0009:150:1"
+# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + test_modprobe() { if [ ! -d $DIR ]; then echo "$0: $DIR not present" >&2 echo "You must have the following enabled in your kernel:" >&2 cat $TEST_DIR/config >&2 - exit 1 + exit $ksft_skip fi }
@@ -105,12 +108,12 @@ test_reqs() { if ! which modprobe 2> /dev/null > /dev/null; then echo "$0: You need modprobe installed" >&2 - exit 1 + exit $ksft_skip fi
if ! which kmod 2> /dev/null > /dev/null; then echo "$0: You need kmod installed" >&2 - exit 1 + exit $ksft_skip fi
# kmod 19 has a bad bug where it returns 0 when modprobe @@ -124,13 +127,13 @@ test_reqs() echo "$0: You need at least kmod 20" >&2 echo "kmod <= 19 is buggy, for details see:" >&2 echo "http://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/libkmod/libkmod..." >&2 - exit 1 + exit $ksft_skip fi
uid=$(id -u) if [ $uid -ne 0 ]; then echo $msg must be run as root >&2 - exit 0 + exit $ksft_skip fi }
On Fri, May 04, 2018 at 07:13:17PM -0600, Shuah Khan (Samsung OSG) wrote:
When kmod test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run. It returns fail in some cases when test is skipped. Either way, it is incorrect and incosnistent reporting.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org
Reviewed-by: Luis R. Rodriguez mcgrof@kernel.org
Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
When kvm test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Change it to use ksft_exit_skip() when the test is skipped. In addition, refine test_assert() message to include strerror() string and add explicit check for root user to clearly identofy non-root user skip case.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/kvm/lib/assert.c | 10 ++++++++-- tools/testing/selftests/kvm/vmx_tsc_adjust_test.c | 4 +++- 2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/assert.c b/tools/testing/selftests/kvm/lib/assert.c index c9f5b7d4ce38..4705729d847e 100644 --- a/tools/testing/selftests/kvm/lib/assert.c +++ b/tools/testing/selftests/kvm/lib/assert.c @@ -13,6 +13,8 @@ #include <execinfo.h> #include <sys/syscall.h>
+#include "../../kselftest.h" + /* Dumps the current stack trace to stderr. */ static void __attribute__((noinline)) test_dump_stack(void); static void test_dump_stack(void) @@ -65,13 +67,17 @@ test_assert(bool exp, const char *exp_str, { va_list ap;
+ if (getuid() != 0) + ksft_exit_skip("Please run the test as root - Exiting.\n"); + if (!(exp)) { va_start(ap, fmt);
fprintf(stderr, "==== Test Assertion Failure ====\n" " %s:%u: %s\n" - " pid=%d tid=%d\n", - file, line, exp_str, getpid(), gettid()); + " pid=%d tid=%d - %s\n", + file, line, exp_str, getpid(), gettid(), + strerror(errno)); test_dump_stack(); if (fmt) { fputs(" ", stderr); diff --git a/tools/testing/selftests/kvm/vmx_tsc_adjust_test.c b/tools/testing/selftests/kvm/vmx_tsc_adjust_test.c index 8f7f62093add..62fb73699eb6 100644 --- a/tools/testing/selftests/kvm/vmx_tsc_adjust_test.c +++ b/tools/testing/selftests/kvm/vmx_tsc_adjust_test.c @@ -28,6 +28,8 @@ #include <string.h> #include <sys/ioctl.h>
+#include "../kselftest.h" + #ifndef MSR_IA32_TSC_ADJUST #define MSR_IA32_TSC_ADJUST 0x3b #endif @@ -190,7 +192,7 @@ int main(int argc, char *argv[])
if (!(entry->ecx & CPUID_VMX)) { printf("nested VMX not enabled, skipping test"); - return 0; + return KSFT_SKIP; }
vm = vm_create_default_vmx(VCPU_ID, (void *) l1_guest_code);
On 05/05/2018 03:13, Shuah Khan (Samsung OSG) wrote:
When kvm test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Change it to use ksft_exit_skip() when the test is skipped. In addition, refine test_assert() message to include strerror() string and add explicit check for root user to clearly identofy non-root user skip case.
Root should not be needed.
Otherwise
Acked-by: Paolo Bonzini pbonzini@redhat.com
Paolo
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org
tools/testing/selftests/kvm/lib/assert.c | 10 ++++++++-- tools/testing/selftests/kvm/vmx_tsc_adjust_test.c | 4 +++- 2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/assert.c b/tools/testing/selftests/kvm/lib/assert.c index c9f5b7d4ce38..4705729d847e 100644 --- a/tools/testing/selftests/kvm/lib/assert.c +++ b/tools/testing/selftests/kvm/lib/assert.c @@ -13,6 +13,8 @@ #include <execinfo.h> #include <sys/syscall.h> +#include "../../kselftest.h"
/* Dumps the current stack trace to stderr. */ static void __attribute__((noinline)) test_dump_stack(void); static void test_dump_stack(void) @@ -65,13 +67,17 @@ test_assert(bool exp, const char *exp_str, { va_list ap;
- if (getuid() != 0)
ksft_exit_skip("Please run the test as root - Exiting.\n");
This should not be needed. Otherwise
if (!(exp)) { va_start(ap, fmt); fprintf(stderr, "==== Test Assertion Failure ====\n" " %s:%u: %s\n"
" pid=%d tid=%d\n",
file, line, exp_str, getpid(), gettid());
" pid=%d tid=%d - %s\n",
file, line, exp_str, getpid(), gettid(),
test_dump_stack(); if (fmt) { fputs(" ", stderr);strerror(errno));
diff --git a/tools/testing/selftests/kvm/vmx_tsc_adjust_test.c b/tools/testing/selftests/kvm/vmx_tsc_adjust_test.c index 8f7f62093add..62fb73699eb6 100644 --- a/tools/testing/selftests/kvm/vmx_tsc_adjust_test.c +++ b/tools/testing/selftests/kvm/vmx_tsc_adjust_test.c @@ -28,6 +28,8 @@ #include <string.h> #include <sys/ioctl.h> +#include "../kselftest.h"
#ifndef MSR_IA32_TSC_ADJUST #define MSR_IA32_TSC_ADJUST 0x3b #endif @@ -190,7 +192,7 @@ int main(int argc, char *argv[]) if (!(entry->ecx & CPUID_VMX)) { printf("nested VMX not enabled, skipping test");
return 0;
}return KSFT_SKIP;
vm = vm_create_default_vmx(VCPU_ID, (void *) l1_guest_code);
-- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/07/2018 05:46 AM, Paolo Bonzini wrote:
On 05/05/2018 03:13, Shuah Khan (Samsung OSG) wrote:
When kvm test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Change it to use ksft_exit_skip() when the test is skipped. In addition, refine test_assert() message to include strerror() string and add explicit check for root user to clearly identofy non-root user skip case.
Root should not be needed.
Will remove the root check and send v2.
Otherwise
Acked-by: Paolo Bonzini pbonzini@redhat.com
thanks, -- Shuah
-- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/07/2018 01:41 PM, Shuah Khan wrote:
On 05/07/2018 05:46 AM, Paolo Bonzini wrote:
On 05/05/2018 03:13, Shuah Khan (Samsung OSG) wrote:
When kvm test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Change it to use ksft_exit_skip() when the test is skipped. In addition, refine test_assert() message to include strerror() string and add explicit check for root user to clearly identofy non-root user skip case.
Root should not be needed.
Will remove the root check and send v2.
Hmm. I am seeing
selftests: kvm: sync_regs_test ======================================== ==== Test Assertion Failure ==== lib/kvm_util.c:54: kvm_fd >= 0 pid=2840 tid=2840 - Permission denied 1 0x0000564cd5206163: ?? ??:0 2 0x0000564cd520531b: ?? ??:0 3 0x00007f7ec018f1c0: ?? ??:0 4 0x0000564cd52058f9: ?? ??:0 open /dev/kvm failed, rc: -1 errno: 13
There are a couple of other tests that fail with EACCES It would make sense report these tests as Skipped perhaps.
thanks, -- Shuah -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
prime_numbers.sh is not included in TEST_PROGS. Add it.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/lib/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/lib/Makefile b/tools/testing/selftests/lib/Makefile index 08360060ab14..70d5711e3ac8 100644 --- a/tools/testing/selftests/lib/Makefile +++ b/tools/testing/selftests/lib/Makefile @@ -3,6 +3,6 @@ # No binaries, but make sure arg-less "make" doesn't trigger "run_tests" all:
-TEST_PROGS := printf.sh bitmap.sh +TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh
include ../lib.mk
When lib test(s) is skipped because of unmet dependencies and/or unsupported configuration, it returns non-zero value hich is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/lib/bitmap.sh | 8 ++++++-- tools/testing/selftests/lib/prime_numbers.sh | 7 +++++-- tools/testing/selftests/lib/printf.sh | 8 ++++++-- 3 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/lib/bitmap.sh b/tools/testing/selftests/lib/bitmap.sh index 4dee4d2a8bbe..5a90006d1aea 100755 --- a/tools/testing/selftests/lib/bitmap.sh +++ b/tools/testing/selftests/lib/bitmap.sh @@ -1,9 +1,13 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0 + +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + # Runs bitmap infrastructure tests using test_bitmap kernel module if ! /sbin/modprobe -q -n test_bitmap; then - echo "bitmap: [SKIP]" - exit 77 + echo "bitmap: module test_bitmap is not found [SKIP]" + exit $ksft_skip fi
if /sbin/modprobe -q test_bitmap; then diff --git a/tools/testing/selftests/lib/prime_numbers.sh b/tools/testing/selftests/lib/prime_numbers.sh index b363994e5e11..76602d4b050f 100755 --- a/tools/testing/selftests/lib/prime_numbers.sh +++ b/tools/testing/selftests/lib/prime_numbers.sh @@ -2,9 +2,12 @@ # SPDX-License-Identifier: GPL-2.0 # Checks fast/slow prime_number generation for inconsistencies
+# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + if ! /sbin/modprobe -q -r prime_numbers; then - echo "prime_numbers: [SKIP]" - exit 77 + echo "prime_numbers: module prime_numbers is not found [SKIP]" + exit $ksft_skip fi
if /sbin/modprobe -q prime_numbers selftest=65536; then diff --git a/tools/testing/selftests/lib/printf.sh b/tools/testing/selftests/lib/printf.sh index 0c37377fd7d4..45a23e2d64ad 100755 --- a/tools/testing/selftests/lib/printf.sh +++ b/tools/testing/selftests/lib/printf.sh @@ -1,9 +1,13 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0 # Runs printf infrastructure using test_printf kernel module + +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + if ! /sbin/modprobe -q -n test_printf; then - echo "printf: [SKIP]" - exit 77 + echo "printf: module test_printf is not found [SKIP]" + exit $ksft_skip fi
if /sbin/modprobe -q test_printf; then
Add Makefile for locking test.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/locking/Makefile | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 tools/testing/selftests/locking/Makefile
diff --git a/tools/testing/selftests/locking/Makefile b/tools/testing/selftests/locking/Makefile new file mode 100644 index 000000000000..e168a2e34bc2 --- /dev/null +++ b/tools/testing/selftests/locking/Makefile @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# Makefile for locking/ww_mutx selftests + +# No binaries, but make sure arg-less "make" doesn't trigger "run_tests" +all: + +TEST_PROGS := ww_mutex.sh + +include ../lib.mk
When locking test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Added an explicit search for ww_mutex module and return skip code if it isn't found to differentiate between the failure to load the module condition and module not found condition.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/locking/ww_mutex.sh | 8 ++++++++ 1 file changed, 8 insertions(+) mode change 100644 => 100755 tools/testing/selftests/locking/ww_mutex.sh
diff --git a/tools/testing/selftests/locking/ww_mutex.sh b/tools/testing/selftests/locking/ww_mutex.sh old mode 100644 new mode 100755 index 2c3d6b1878c2..91e4ac7566af --- a/tools/testing/selftests/locking/ww_mutex.sh +++ b/tools/testing/selftests/locking/ww_mutex.sh @@ -1,6 +1,14 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0 + +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + # Runs API tests for struct ww_mutex (Wait/Wound mutexes) +if ! /sbin/modprobe -q -n test-ww_mutex; then + echo "ww_mutex: module test-ww_mutex is not found [SKIP]" + exit $ksft_skip +fi
if /sbin/modprobe -q test-ww_mutex; then /sbin/modprobe -q -r test-ww_mutex
When media_tests test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Change it to use ksft_exit_skip() when the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/media_tests/Makefile | 3 ++- tools/testing/selftests/media_tests/media_device_open.c | 8 ++++---- tools/testing/selftests/media_tests/media_device_test.c | 8 ++++---- 3 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/media_tests/Makefile b/tools/testing/selftests/media_tests/Makefile index c82cec2497de..60826d7d37d4 100644 --- a/tools/testing/selftests/media_tests/Makefile +++ b/tools/testing/selftests/media_tests/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 +# +CFLAGS += -I../ -I../../../../usr/include/ TEST_GEN_PROGS := media_device_test media_device_open video_device_test -all: $(TEST_GEN_PROGS)
include ../lib.mk diff --git a/tools/testing/selftests/media_tests/media_device_open.c b/tools/testing/selftests/media_tests/media_device_open.c index a5ce5434bafd..93183a37b133 100644 --- a/tools/testing/selftests/media_tests/media_device_open.c +++ b/tools/testing/selftests/media_tests/media_device_open.c @@ -34,6 +34,8 @@ #include <sys/stat.h> #include <linux/media.h>
+#include "../kselftest.h" + int main(int argc, char **argv) { int opt; @@ -61,10 +63,8 @@ int main(int argc, char **argv) } }
- if (getuid() != 0) { - printf("Please run the test as root - Exiting.\n"); - exit(-1); - } + if (getuid() != 0) + ksft_exit_skip("Please run the test as root - Exiting.\n");
/* Open Media device and keep it open */ fd = open(media_device, O_RDWR); diff --git a/tools/testing/selftests/media_tests/media_device_test.c b/tools/testing/selftests/media_tests/media_device_test.c index 421a367e4bb3..a4c6ad18cb5c 100644 --- a/tools/testing/selftests/media_tests/media_device_test.c +++ b/tools/testing/selftests/media_tests/media_device_test.c @@ -39,6 +39,8 @@ #include <time.h> #include <linux/media.h>
+#include "../kselftest.h" + int main(int argc, char **argv) { int opt; @@ -66,10 +68,8 @@ int main(int argc, char **argv) } }
- if (getuid() != 0) { - printf("Please run the test as root - Exiting.\n"); - exit(-1); - } + if (getuid() != 0) + ksft_exit_skip("Please run the test as root - Exiting.\n");
/* Generate random number of interations */ srand((unsigned int) time(NULL));
When membarrier test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Change it to use ksft_exit_skip() when the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/membarrier/membarrier_test.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/membarrier/membarrier_test.c b/tools/testing/selftests/membarrier/membarrier_test.c index 22bffd55a523..6793f8ecc8e7 100644 --- a/tools/testing/selftests/membarrier/membarrier_test.c +++ b/tools/testing/selftests/membarrier/membarrier_test.c @@ -293,10 +293,9 @@ static int test_membarrier_query(void) } ksft_exit_fail_msg("sys_membarrier() failed\n"); } - if (!(ret & MEMBARRIER_CMD_GLOBAL)) { - ksft_test_result_fail("sys_membarrier() CMD_GLOBAL query failed\n"); - ksft_exit_fail_msg("sys_membarrier is not supported.\n"); - } + if (!(ret & MEMBARRIER_CMD_GLOBAL)) + ksft_exit_skip( + "sys_membarrier unsupported: CMD_GLOBAL not found.\n");
ksft_test_result_pass("sys_membarrier available\n"); return 0;
When memfd test is skipped because of unmet dependencies and/or unsupported configuration, it returns non-zero value which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Added an explicit check for root user and return skip code.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/memfd/run_tests.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/memfd/run_tests.sh b/tools/testing/selftests/memfd/run_tests.sh index c2d41ed81b24..88dc206a69b7 100755 --- a/tools/testing/selftests/memfd/run_tests.sh +++ b/tools/testing/selftests/memfd/run_tests.sh @@ -1,6 +1,14 @@ #!/bin/bash # please run as root
+# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + +if [ $UID != 0 ]; then + echo "Please run this test as root" + exit $ksft_skip +fi + # # Normal tests requiring no special resources # @@ -33,7 +41,7 @@ if [ -n "$freepgs" ] && [ $freepgs -lt $hpages_test ]; then echo $(( $hpages_needed + $nr_hugepgs )) > /proc/sys/vm/nr_hugepages if [ $? -ne 0 ]; then echo "Please run this test as root" - exit 1 + exit $ksft_skip fi while read name size unit; do if [ "$name" = "HugePages_Free:" ]; then
On 05/04/2018 06:13 PM, Shuah Khan (Samsung OSG) wrote:
When memfd test is skipped because of unmet dependencies and/or unsupported configuration, it returns non-zero value which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Added an explicit check for root user and return skip code.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org
tools/testing/selftests/memfd/run_tests.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/memfd/run_tests.sh b/tools/testing/selftests/memfd/run_tests.sh index c2d41ed81b24..88dc206a69b7 100755 --- a/tools/testing/selftests/memfd/run_tests.sh +++ b/tools/testing/selftests/memfd/run_tests.sh @@ -1,6 +1,14 @@ #!/bin/bash # please run as root +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4
+if [ $UID != 0 ]; then
- echo "Please run this test as root"
- exit $ksft_skip
+fi
# # Normal tests requiring no special resources # @@ -33,7 +41,7 @@ if [ -n "$freepgs" ] && [ $freepgs -lt $hpages_test ]; then echo $(( $hpages_needed + $nr_hugepgs )) > /proc/sys/vm/nr_hugepages if [ $? -ne 0 ]; then echo "Please run this test as root"
exit 1
exit $ksft_skip
We now KNOW that we are running as root because of the check above. We can delete this test, and rely on the later check to determine if the number of huge pages was actually increased.
How about this instead (untested)?
Signed-off-by: Mike Kravetz mike.kravetz@oracle.com diff --git a/tools/testing/selftests/memfd/run_tests.sh b/tools/testing/selftests/memfd/run_tests.sh index c2d41ed81b24..99a265a84e1d 100755 --- a/tools/testing/selftests/memfd/run_tests.sh +++ b/tools/testing/selftests/memfd/run_tests.sh @@ -1,6 +1,14 @@ #!/bin/bash # please run as root
+# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + +if [ $UID != 0 ]; then + echo "Please run this test as root" + exit $ksft_skip +fi + # # Normal tests requiring no special resources # @@ -31,10 +39,6 @@ if [ -n "$freepgs" ] && [ $freepgs -lt $hpages_test ]; then
echo 3 > /proc/sys/vm/drop_caches echo $(( $hpages_needed + $nr_hugepgs )) > /proc/sys/vm/nr_hugepages - if [ $? -ne 0 ]; then - echo "Please run this test as root" - exit 1 - fi while read name size unit; do if [ "$name" = "HugePages_Free:" ]; then freepgs=$size @@ -53,7 +57,7 @@ if [ $freepgs -lt $hpages_test ]; then fi printf "Not enough huge pages available (%d < %d)\n" \ $freepgs $needpgs - exit 1 + exit $ksft_skip fi
# -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/04/2018 07:52 PM, Mike Kravetz wrote:
On 05/04/2018 06:13 PM, Shuah Khan (Samsung OSG) wrote:
When memfd test is skipped because of unmet dependencies and/or unsupported configuration, it returns non-zero value which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Added an explicit check for root user and return skip code.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org
tools/testing/selftests/memfd/run_tests.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/memfd/run_tests.sh b/tools/testing/selftests/memfd/run_tests.sh index c2d41ed81b24..88dc206a69b7 100755 --- a/tools/testing/selftests/memfd/run_tests.sh +++ b/tools/testing/selftests/memfd/run_tests.sh @@ -1,6 +1,14 @@ #!/bin/bash # please run as root +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4
+if [ $UID != 0 ]; then
- echo "Please run this test as root"
- exit $ksft_skip
+fi
# # Normal tests requiring no special resources # @@ -33,7 +41,7 @@ if [ -n "$freepgs" ] && [ $freepgs -lt $hpages_test ]; then echo $(( $hpages_needed + $nr_hugepgs )) > /proc/sys/vm/nr_hugepages if [ $? -ne 0 ]; then echo "Please run this test as root"
exit 1
exit $ksft_skip
We now KNOW that we are running as root because of the check above. We can delete this test, and rely on the later check to determine if the number of huge pages was actually increased.
Thanks for the review. Yesh this check can go away.
How about this instead (untested)?
Signed-off-by: Mike Kravetz mike.kravetz@oracle.com diff --git a/tools/testing/selftests/memfd/run_tests.sh b/tools/testing/selftests/memfd/run_tests.sh index c2d41ed81b24..99a265a84e1d 100755 --- a/tools/testing/selftests/memfd/run_tests.sh +++ b/tools/testing/selftests/memfd/run_tests.sh @@ -1,6 +1,14 @@ #!/bin/bash # please run as root +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4
+if [ $UID != 0 ]; then
- echo "Please run this test as root"
- exit $ksft_skip
+fi
# # Normal tests requiring no special resources # @@ -31,10 +39,6 @@ if [ -n "$freepgs" ] && [ $freepgs -lt $hpages_test ]; then echo 3 > /proc/sys/vm/drop_caches echo $(( $hpages_needed + $nr_hugepgs )) > /proc/sys/vm/nr_hugepages
- if [ $? -ne 0 ]; then
echo "Please run this test as root"
exit 1
- fi while read name size unit; do if [ "$name" = "HugePages_Free:" ]; then freepgs=$size
@@ -53,7 +57,7 @@ if [ $freepgs -lt $hpages_test ]; then fi printf "Not enough huge pages available (%d < %d)\n" \ $freepgs $needpgs
- exit 1
- exit $ksft_skip
fi
I thought about changing this to skip and wasn't sure since the test did run partially. Good to know this case can be classified as skip. I will make the suggested changes and send v2.
thanks, -- Shuah -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/07/2018 12:45 PM, Shuah Khan wrote:
On 05/04/2018 07:52 PM, Mike Kravetz wrote:
On 05/04/2018 06:13 PM, Shuah Khan (Samsung OSG) wrote:
When memfd test is skipped because of unmet dependencies and/or unsupported configuration, it returns non-zero value which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Added an explicit check for root user and return skip code.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org
tools/testing/selftests/memfd/run_tests.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/memfd/run_tests.sh b/tools/testing/selftests/memfd/run_tests.sh index c2d41ed81b24..88dc206a69b7 100755 --- a/tools/testing/selftests/memfd/run_tests.sh +++ b/tools/testing/selftests/memfd/run_tests.sh @@ -1,6 +1,14 @@ #!/bin/bash # please run as root +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4
+if [ $UID != 0 ]; then
- echo "Please run this test as root"
- exit $ksft_skip
+fi
Moving root check this far up messes the test up and requires root access for run_fuse_test.sh and memfd_test which is incorrect.
I will re-do the patch
thanks, -- Shuah -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
When memory-hotplug test is skipped because of unmet dependencies and/or unsupported configuration, it returns non-zero value hich is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/memory-hotplug/mem-on-off-test.sh | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh index ff4991704d07..b37585e6aa38 100755 --- a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh +++ b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh @@ -3,30 +3,33 @@
SYSFS=
+# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + prerequisite() { msg="skip all tests:"
if [ $UID != 0 ]; then echo $msg must be run as root >&2 - exit 0 + exit $ksft_skip fi
SYSFS=`mount -t sysfs | head -1 | awk '{ print $3 }'`
if [ ! -d "$SYSFS" ]; then echo $msg sysfs is not mounted >&2 - exit 0 + exit $ksft_skip fi
if ! ls $SYSFS/devices/system/memory/memory* > /dev/null 2>&1; then echo $msg memory hotplug is not supported >&2 - exit 0 + exit $ksft_skip fi
if ! grep -q 1 $SYSFS/devices/system/memory/memory*/removable; then echo $msg no hot-pluggable memory >&2 - exit 0 + exit $ksft_skip fi }
When mqueue test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Change it to use ksft_exit_skip() when the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/mqueue/mq_open_tests.c | 8 ++++---- tools/testing/selftests/mqueue/mq_perf_tests.c | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/mqueue/mq_open_tests.c b/tools/testing/selftests/mqueue/mq_open_tests.c index 677140aa25fd..9403ac01ba11 100644 --- a/tools/testing/selftests/mqueue/mq_open_tests.c +++ b/tools/testing/selftests/mqueue/mq_open_tests.c @@ -33,6 +33,8 @@ #include <mqueue.h> #include <error.h>
+#include "../kselftest.h" + static char *usage = "Usage:\n" " %s path\n" @@ -262,12 +264,10 @@ int main(int argc, char *argv[]) } }
- if (getuid() != 0) { - fprintf(stderr, "Not running as root, but almost all tests " + if (getuid() != 0) + ksft_exit_skip("Not running as root, but almost all tests " "require root in order to modify\nsystem settings. " "Exiting.\n"); - exit(1); - }
/* Find out what files there are for us to make tweaks in */ def_msgs = fopen(DEF_MSGS, "r+"); diff --git a/tools/testing/selftests/mqueue/mq_perf_tests.c b/tools/testing/selftests/mqueue/mq_perf_tests.c index 8188f72de93c..b019e0b8221c 100644 --- a/tools/testing/selftests/mqueue/mq_perf_tests.c +++ b/tools/testing/selftests/mqueue/mq_perf_tests.c @@ -39,6 +39,8 @@ #include <popt.h> #include <error.h>
+#include "../kselftest.h" + static char *usage = "Usage:\n" " %s [-c #[,#..] -f] path\n" @@ -626,12 +628,10 @@ int main(int argc, char *argv[]) cpus_to_pin[0] = cpus_online - 1; }
- if (getuid() != 0) { - fprintf(stderr, "Not running as root, but almost all tests " + if (getuid() != 0) + ksft_exit_skip("Not running as root, but almost all tests " "require root in order to modify\nsystem settings. " "Exiting.\n"); - exit(1); - }
max_msgs = fopen(MAX_MSGS, "r+"); max_msgsize = fopen(MAX_MSGSIZE, "r+");
When net test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Change psock_tpacket to use ksft_exit_skip() when a non-root user runs the test and add an explicit check for root and a clear message, instead of failing the test when /sys/power/state file open fails.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org --- tools/testing/selftests/net/fib_tests.sh | 8 +++++--- tools/testing/selftests/net/netdevice.sh | 16 +++++++++------ tools/testing/selftests/net/pmtu.sh | 5 ++++- tools/testing/selftests/net/psock_tpacket.c | 4 +++- tools/testing/selftests/net/rtnetlink.sh | 31 ++++++++++++++++------------- 5 files changed, 39 insertions(+), 25 deletions(-)
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh index 9164e60d4b66..5baac82b9287 100755 --- a/tools/testing/selftests/net/fib_tests.sh +++ b/tools/testing/selftests/net/fib_tests.sh @@ -5,6 +5,8 @@ # different events.
ret=0 +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4
VERBOSE=${VERBOSE:=0} PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no} @@ -579,18 +581,18 @@ fib_test()
if [ "$(id -u)" -ne 0 ];then echo "SKIP: Need root privileges" - exit 0 + exit $ksft_skip; fi
if [ ! -x "$(command -v ip)" ]; then echo "SKIP: Could not run test without ip tool" - exit 0 + exit $ksft_skip fi
ip route help 2>&1 | grep -q fibmatch if [ $? -ne 0 ]; then echo "SKIP: iproute2 too old, missing fibmatch" - exit 0 + exit $ksft_skip fi
# start clean diff --git a/tools/testing/selftests/net/netdevice.sh b/tools/testing/selftests/net/netdevice.sh index 903679e0ff31..e3afcb424710 100755 --- a/tools/testing/selftests/net/netdevice.sh +++ b/tools/testing/selftests/net/netdevice.sh @@ -8,6 +8,9 @@ # if not they probably have failed earlier in the boot process and their logged error will be catched by another test #
+# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + # this function will try to up the interface # if already up, nothing done # arg1: network interface name @@ -18,7 +21,7 @@ kci_net_start() ip link show "$netdev" |grep -q UP if [ $? -eq 0 ];then echo "SKIP: $netdev: interface already up" - return 0 + return $ksft_skip fi
ip link set "$netdev" up @@ -61,12 +64,12 @@ kci_net_setup() ip address show "$netdev" |grep '^[[:space:]]*inet' if [ $? -eq 0 ];then echo "SKIP: $netdev: already have an IP" - return 0 + return $ksft_skip fi
# TODO what ipaddr to set ? DHCP ? echo "SKIP: $netdev: set IP address" - return 0 + return $ksft_skip }
# test an ethtool command @@ -84,6 +87,7 @@ kci_netdev_ethtool_test() if [ $ret -ne 0 ];then if [ $ret -eq "$1" ];then echo "SKIP: $netdev: ethtool $2 not supported" + return $ksft_skip else echo "FAIL: $netdev: ethtool $2" return 1 @@ -104,7 +108,7 @@ kci_netdev_ethtool() ethtool --version 2>/dev/null >/dev/null if [ $? -ne 0 ];then echo "SKIP: ethtool not present" - return 1 + return $ksft_skip fi
TMP_ETHTOOL_FEATURES="$(mktemp)" @@ -176,13 +180,13 @@ kci_test_netdev() #check for needed privileges if [ "$(id -u)" -ne 0 ];then echo "SKIP: Need root privileges" - exit 0 + exit $ksft_skip fi
ip link show 2>/dev/null >/dev/null if [ $? -ne 0 ];then echo "SKIP: Could not run test without the ip tool" - exit 0 + exit $ksft_skip fi
TMP_LIST_NETDEV="$(mktemp)" diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh index 1e428781a625..7514f93e1624 100755 --- a/tools/testing/selftests/net/pmtu.sh +++ b/tools/testing/selftests/net/pmtu.sh @@ -43,6 +43,9 @@ # that MTU is properly calculated instead when MTU is not configured from # userspace
+# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + tests=" pmtu_vti6_exception vti6: PMTU exceptions pmtu_vti4_exception vti4: PMTU exceptions @@ -162,7 +165,7 @@ setup_xfrm6() { }
setup() { - [ "$(id -u)" -ne 0 ] && echo " need to run as root" && return 1 + [ "$(id -u)" -ne 0 ] && echo " need to run as root" && return $ksft_skip
cleanup_done=0 for arg do diff --git a/tools/testing/selftests/net/psock_tpacket.c b/tools/testing/selftests/net/psock_tpacket.c index 7f6cd9fdacf3..7ec4fa4d55dc 100644 --- a/tools/testing/selftests/net/psock_tpacket.c +++ b/tools/testing/selftests/net/psock_tpacket.c @@ -60,6 +60,8 @@
#include "psock_lib.h"
+#include "../kselftest.h" + #ifndef bug_on # define bug_on(cond) assert(!(cond)) #endif @@ -825,7 +827,7 @@ static int test_tpacket(int version, int type) fprintf(stderr, "test: skip %s %s since user and kernel " "space have different bit width\n", tpacket_str[version], type_str[type]); - return 0; + return KSFT_SKIP; }
sock = pfsocket(version); diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh index e6f485235435..fb3767844e42 100755 --- a/tools/testing/selftests/net/rtnetlink.sh +++ b/tools/testing/selftests/net/rtnetlink.sh @@ -7,6 +7,9 @@ devdummy="test-dummy0" ret=0
+# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + # set global exit status, but never reset nonzero one. check_err() { @@ -333,7 +336,7 @@ kci_test_vrf() ip link show type vrf 2>/dev/null if [ $? -ne 0 ]; then echo "SKIP: vrf: iproute2 too old" - return 0 + return $ksft_skip fi
ip link add "$vrfname" type vrf table 10 @@ -409,7 +412,7 @@ kci_test_encap_fou() ip fou help 2>&1 |grep -q 'Usage: ip fou' if [ $? -ne 0 ];then echo "SKIP: fou: iproute2 too old" - return 1 + return $ksft_skip fi
ip netns exec "$testns" ip fou add port 7777 ipproto 47 2>/dev/null @@ -444,7 +447,7 @@ kci_test_encap() ip netns add "$testns" if [ $? -ne 0 ]; then echo "SKIP encap tests: cannot add net namespace $testns" - return 1 + return $ksft_skip fi
ip netns exec "$testns" ip link set lo up @@ -469,7 +472,7 @@ kci_test_macsec() ip macsec help 2>&1 | grep -q "^Usage: ip macsec" if [ $? -ne 0 ]; then echo "SKIP: macsec: iproute2 too old" - return 0 + return $ksft_skip fi
ip link add link "$devdummy" "$msname" type macsec port 42 encrypt on @@ -511,14 +514,14 @@ kci_test_gretap() ip netns add "$testns" if [ $? -ne 0 ]; then echo "SKIP gretap tests: cannot add net namespace $testns" - return 1 + return $ksft_skip fi
ip link help gretap 2>&1 | grep -q "^Usage:" if [ $? -ne 0 ];then echo "SKIP: gretap: iproute2 too old" ip netns del "$testns" - return 1 + return $ksft_skip fi
# test native tunnel @@ -561,14 +564,14 @@ kci_test_ip6gretap() ip netns add "$testns" if [ $? -ne 0 ]; then echo "SKIP ip6gretap tests: cannot add net namespace $testns" - return 1 + return $ksft_skip fi
ip link help ip6gretap 2>&1 | grep -q "^Usage:" if [ $? -ne 0 ];then echo "SKIP: ip6gretap: iproute2 too old" ip netns del "$testns" - return 1 + return $ksft_skip fi
# test native tunnel @@ -611,13 +614,13 @@ kci_test_erspan() ip link help erspan 2>&1 | grep -q "^Usage:" if [ $? -ne 0 ];then echo "SKIP: erspan: iproute2 too old" - return 1 + return $ksft_skip fi
ip netns add "$testns" if [ $? -ne 0 ]; then echo "SKIP erspan tests: cannot add net namespace $testns" - return 1 + return $ksft_skip fi
# test native tunnel erspan v1 @@ -676,13 +679,13 @@ kci_test_ip6erspan() ip link help ip6erspan 2>&1 | grep -q "^Usage:" if [ $? -ne 0 ];then echo "SKIP: ip6erspan: iproute2 too old" - return 1 + return $ksft_skip fi
ip netns add "$testns" if [ $? -ne 0 ]; then echo "SKIP ip6erspan tests: cannot add net namespace $testns" - return 1 + return $ksft_skip fi
# test native tunnel ip6erspan v1 @@ -762,14 +765,14 @@ kci_test_rtnl() #check for needed privileges if [ "$(id -u)" -ne 0 ];then echo "SKIP: Need root privileges" - exit 0 + exit $ksft_skip fi
for x in ip tc;do $x -Version 2>/dev/null >/dev/null if [ $? -ne 0 ];then echo "SKIP: Could not run test without the $x tool" - exit 0 + exit $ksft_skip fi done
On Sat, May 5, 2018 at 6:43 AM, Shuah Khan (Samsung OSG) shuah@kernel.org wrote:
When ion test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run.
Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run.
Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped.
Signed-off-by: Shuah Khan (Samsung OSG) shuah@kernel.org
tools/testing/selftests/android/ion/ion_test.sh | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/android/ion/ion_test.sh b/tools/testing/selftests/android/ion/ion_test.sh index a1aff506f5e6..69e676cfc94e 100755 --- a/tools/testing/selftests/android/ion/ion_test.sh +++ b/tools/testing/selftests/android/ion/ion_test.sh @@ -4,6 +4,9 @@ heapsize=4096 TCID="ion_test.sh" errcode=0
+# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4
run_test() { heaptype=$1 @@ -25,7 +28,7 @@ check_root() uid=$(id -u) if [ $uid -ne 0 ]; then echo $TCID: must be run as root >&2
exit 0
exit $ksft_skip fi
}
@@ -35,7 +38,7 @@ check_device() if [ ! -e $DEVICE ]; then echo $TCID: No $DEVICE device found >&2 echo $TCID: May be CONFIG_ION is not set >&2
exit 0
exit $ksft_skip fi
}
Ok changes looks good to me.
Acked-by: Pintu Agarwal pintu.ping@gmail.com
-- 2.14.1
-- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
linux-kselftest-mirror@lists.linaro.org