On 2/15/22 7:26 PM, Shaopeng Tan wrote:
In kselftest framework, if a sub test can not run by some reasons, the test result should be marked as SKIP rather than FAIL. Return KSFT_SKIP(4) instead of KSFT_FAIL(1) if resctrl_tests is not run as root or it is run on a test environment which does not support resctrl.
- ksft_exit_fail_msg(): returns KSFT_FAIL(1)
- ksft_exit_skip(): returns KSFT_SKIP(4)
Thank for making this change to skip.
Signed-off-by: Shaopeng Tan tan.shaopeng@jp.fujitsu.com
Some important feedbacks from v1&v2 are addressed as follows:
It is important to highlight that a skipped test is marked as successful to not unnecessarily report a feature failure when there actually is a failure in the test environment. => I highligted this content in changelog as follows. "In kselftest framework, if a test is not run by some reason, the test result is marked as SKIP rather than FAIL."
The subject line can be made more succinct while the details are moved to the commit message. => I made subject line succinct and moved the details to changelog.
How changing ksft_exit_fail_msg() to ksft_exit_skip() accomplishes the goal of unifying the return code. => In changelog, I explained why return code KSFT_SKIP(4) is needed in kselftest framework and the return code of each function.
tools/testing/selftests/resctrl/resctrl_tests.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index 973f09a66e1e..3be0895c492b 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -205,7 +205,7 @@ int main(int argc, char **argv) * 2. We execute perf commands */ if (geteuid() != 0)
return ksft_exit_fail_msg("Not running as root, abort testing.\n");
return ksft_exit_skip("Not running as root, abort testing.\n");
Let's update the message to "This test must be run as root. Skipping..."
/* Detect AMD vendor */ detect_amd(); @@ -235,7 +235,7 @@ int main(int argc, char **argv) sprintf(bm_type, "fill_buf"); if (!check_resctrlfs_support())
return ksft_exit_fail_msg("resctrl FS does not exist\n");
return ksft_exit_skip("resctrl FS does not exist\n");
Update the message to read -
"resctrl FS doesn not exist. Enable X86_CPU_RESCTRL and PROC_CPU_RESCTRL config options"
filter_dmesg();
With these changes:
Reviewed-by: Shuah Khan skhan@linuxfoundation.org
thanks, -- Shuah