| Message ID | 20241122155548.55495-1-laura.nao@collabora.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests: Warn about skipped tests in result summary | expand |
On 11/22/24 08:55, Laura Nao wrote: > Update the functions that print the test totals at the end of a selftest > to include a warning message when skipped tests are detected. The > message advises users that skipped tests may indicate missing > configuration options and suggests enabling them to improve coverage. > > Signed-off-by: Laura Nao <laura.nao@collabora.com> > --- > This patch follows up on a previous discussion[1] and aims to highlight > skipped tests for the user's attention. > > [1] https://lore.kernel.org/lkml/2bb2d338-cd00-4ac2-b8bd-5579eae82637@linuxfoundation.org/ > --- > tools/testing/selftests/kselftest.h | 4 ++++ > tools/testing/selftests/kselftest/ksft.py | 3 +++ > tools/testing/selftests/kselftest/ktap_helpers.sh | 4 ++++ > 3 files changed, 11 insertions(+) > > diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h > index 29fedf609611..d3f64b333acd 100644 > --- a/tools/testing/selftests/kselftest.h > +++ b/tools/testing/selftests/kselftest.h > @@ -147,6 +147,10 @@ static inline void ksft_set_plan(unsigned int plan) > > static inline void ksft_print_cnts(void) > { > + if (ksft_cnt.ksft_xskip > 0) > + printf( > + "# Skipped tests detected. Consider enabling relevant config options to improve coverage.\n" I like this. How about printing the number of skipped tests in this message also to make it easy to parse. Same comment on other print messages, > + ); > if (ksft_plan != ksft_test_num()) > printf("# Planned tests != run tests (%u != %u)\n", > ksft_plan, ksft_test_num()); > diff --git a/tools/testing/selftests/kselftest/ksft.py b/tools/testing/selftests/kselftest/ksft.py > index bf215790a89d..7675a15a1264 100644 > --- a/tools/testing/selftests/kselftest/ksft.py > +++ b/tools/testing/selftests/kselftest/ksft.py > @@ -27,6 +27,9 @@ def set_plan(num_tests): > > > def print_cnts(): > + if ksft_cnt['skip'] > 0: > + print("# Skipped tests detected. Consider enabling relevant config options to improve coverage.") > + > print( > f"# Totals: pass:{ksft_cnt['pass']} fail:{ksft_cnt['fail']} xfail:0 xpass:0 skip:{ksft_cnt['skip']} error:0" > ) > diff --git a/tools/testing/selftests/kselftest/ktap_helpers.sh b/tools/testing/selftests/kselftest/ktap_helpers.sh > index 79a125eb24c2..a4211221ccd6 100644 > --- a/tools/testing/selftests/kselftest/ktap_helpers.sh > +++ b/tools/testing/selftests/kselftest/ktap_helpers.sh > @@ -107,5 +107,9 @@ ktap_finished() { > } > > ktap_print_totals() { > + if [ "$KTAP_CNT_SKIP" -gt 0 ]; then > + echo "# Skipped tests detected. " \ > + "Consider enabling relevant config options to improve coverage." > + fi > echo "# Totals: pass:$KTAP_CNT_PASS fail:$KTAP_CNT_FAIL xfail:0 xpass:0 skip:$KTAP_CNT_SKIP error:0" > } thanks, -- Shuah
Hi Laura, Thank you for making change. On 11/22/24 11:19 PM, Shuah wrote: > On 11/22/24 08:55, Laura Nao wrote: >> Update the functions that print the test totals at the end of a selftest >> to include a warning message when skipped tests are detected. The >> message advises users that skipped tests may indicate missing >> configuration options and suggests enabling them to improve coverage. >> >> Signed-off-by: Laura Nao <laura.nao@collabora.com> >> --- >> This patch follows up on a previous discussion[1] and aims to highlight >> skipped tests for the user's attention. >> >> [1] https://lore.kernel.org/lkml/2bb2d338-cd00-4ac2-b8bd-5579eae82637@linuxfoundation.org/ >> --- >> tools/testing/selftests/kselftest.h | 4 ++++ >> tools/testing/selftests/kselftest/ksft.py | 3 +++ >> tools/testing/selftests/kselftest/ktap_helpers.sh | 4 ++++ >> 3 files changed, 11 insertions(+) >> >> diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h >> index 29fedf609611..d3f64b333acd 100644 >> --- a/tools/testing/selftests/kselftest.h >> +++ b/tools/testing/selftests/kselftest.h >> @@ -147,6 +147,10 @@ static inline void ksft_set_plan(unsigned int plan) >> static inline void ksft_print_cnts(void) >> { >> + if (ksft_cnt.ksft_xskip > 0) >> + printf( >> + "# Skipped tests detected. Consider enabling relevant config options to improve coverage.\n" Looks good. Printing the number of skipped tests would be an improvement. I'm thinking about a case where some tests got failed and some skipped. Would this warning be useful in that case? > > I like this. How about printing the number of skipped tests in this > message also to make it easy to parse. > > Same comment on other print messages, > >> + ); >> if (ksft_plan != ksft_test_num()) >> printf("# Planned tests != run tests (%u != %u)\n", >> ksft_plan, ksft_test_num()); >> diff --git a/tools/testing/selftests/kselftest/ksft.py b/tools/testing/selftests/kselftest/ksft.py >> index bf215790a89d..7675a15a1264 100644 >> --- a/tools/testing/selftests/kselftest/ksft.py >> +++ b/tools/testing/selftests/kselftest/ksft.py >> @@ -27,6 +27,9 @@ def set_plan(num_tests): >> def print_cnts(): >> + if ksft_cnt['skip'] > 0: >> + print("# Skipped tests detected. Consider enabling relevant config options to improve coverage.") >> + >> print( >> f"# Totals: pass:{ksft_cnt['pass']} fail:{ksft_cnt['fail']} xfail:0 xpass:0 skip:{ksft_cnt['skip']} error:0" >> ) >> diff --git a/tools/testing/selftests/kselftest/ktap_helpers.sh b/tools/testing/selftests/kselftest/ktap_helpers.sh >> index 79a125eb24c2..a4211221ccd6 100644 >> --- a/tools/testing/selftests/kselftest/ktap_helpers.sh >> +++ b/tools/testing/selftests/kselftest/ktap_helpers.sh >> @@ -107,5 +107,9 @@ ktap_finished() { >> } >> ktap_print_totals() { >> + if [ "$KTAP_CNT_SKIP" -gt 0 ]; then >> + echo "# Skipped tests detected. " \ >> + "Consider enabling relevant config options to improve coverage." >> + fi >> echo "# Totals: pass:$KTAP_CNT_PASS fail:$KTAP_CNT_FAIL xfail:0 xpass:0 skip:$KTAP_CNT_SKIP error:0" >> } > > thanks, > -- Shuah >
Hi Shuah and Usama, On 11/25/24 09:46, Muhammad Usama Anjum wrote: > Hi Laura, > > Thank you for making change. > > On 11/22/24 11:19 PM, Shuah wrote: >> On 11/22/24 08:55, Laura Nao wrote: >>> Update the functions that print the test totals at the end of a selftest >>> to include a warning message when skipped tests are detected. The >>> message advises users that skipped tests may indicate missing >>> configuration options and suggests enabling them to improve coverage. >>> >>> Signed-off-by: Laura Nao <laura.nao@collabora.com> >>> --- >>> This patch follows up on a previous discussion[1] and aims to highlight >>> skipped tests for the user's attention. >>> >>> [1] https://lore.kernel.org/lkml/2bb2d338-cd00-4ac2-b8bd-5579eae82637@linuxfoundation.org/ >>> --- >>> tools/testing/selftests/kselftest.h | 4 ++++ >>> tools/testing/selftests/kselftest/ksft.py | 3 +++ >>> tools/testing/selftests/kselftest/ktap_helpers.sh | 4 ++++ >>> 3 files changed, 11 insertions(+) >>> >>> diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h >>> index 29fedf609611..d3f64b333acd 100644 >>> --- a/tools/testing/selftests/kselftest.h >>> +++ b/tools/testing/selftests/kselftest.h >>> @@ -147,6 +147,10 @@ static inline void ksft_set_plan(unsigned int plan) >>> static inline void ksft_print_cnts(void) >>> { >>> + if (ksft_cnt.ksft_xskip > 0) >>> + printf( >>> + "# Skipped tests detected. Consider enabling relevant config options to improve coverage.\n" > Looks good. Printing the number of skipped tests would be an improvement. > I'm thinking about a case where some tests got failed and some skipped. Would > this warning be useful in that case? > I believe the warning remains useful, as it helps users identify possible gaps in their configuration - I think that's valuable regardless of the results of other tests. >> >> I like this. How about printing the number of skipped tests in this >> message also to make it easy to parse. >> >> Same comment on other print messages, >> Sure, that makes sense. I'll submit a v2 to include the number of skipped tests. >>> + ); >>> if (ksft_plan != ksft_test_num()) >>> printf("# Planned tests != run tests (%u != %u)\n", >>> ksft_plan, ksft_test_num()); >>> diff --git a/tools/testing/selftests/kselftest/ksft.py b/tools/testing/selftests/kselftest/ksft.py >>> index bf215790a89d..7675a15a1264 100644 >>> --- a/tools/testing/selftests/kselftest/ksft.py >>> +++ b/tools/testing/selftests/kselftest/ksft.py >>> @@ -27,6 +27,9 @@ def set_plan(num_tests): >>> def print_cnts(): >>> + if ksft_cnt['skip'] > 0: >>> + print("# Skipped tests detected. Consider enabling relevant config options to improve coverage.") >>> + >>> print( >>> f"# Totals: pass:{ksft_cnt['pass']} fail:{ksft_cnt['fail']} xfail:0 xpass:0 skip:{ksft_cnt['skip']} error:0" >>> ) >>> diff --git a/tools/testing/selftests/kselftest/ktap_helpers.sh b/tools/testing/selftests/kselftest/ktap_helpers.sh >>> index 79a125eb24c2..a4211221ccd6 100644 >>> --- a/tools/testing/selftests/kselftest/ktap_helpers.sh >>> +++ b/tools/testing/selftests/kselftest/ktap_helpers.sh >>> @@ -107,5 +107,9 @@ ktap_finished() { >>> } >>> ktap_print_totals() { >>> + if [ "$KTAP_CNT_SKIP" -gt 0 ]; then >>> + echo "# Skipped tests detected. " \ >>> + "Consider enabling relevant config options to improve coverage." >>> + fi >>> echo "# Totals: pass:$KTAP_CNT_PASS fail:$KTAP_CNT_FAIL xfail:0 xpass:0 skip:$KTAP_CNT_SKIP error:0" >>> } >> >> thanks, >> -- Shuah >> > Thanks, Laura
diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h index 29fedf609611..d3f64b333acd 100644 --- a/tools/testing/selftests/kselftest.h +++ b/tools/testing/selftests/kselftest.h @@ -147,6 +147,10 @@ static inline void ksft_set_plan(unsigned int plan) static inline void ksft_print_cnts(void) { + if (ksft_cnt.ksft_xskip > 0) + printf( + "# Skipped tests detected. Consider enabling relevant config options to improve coverage.\n" + ); if (ksft_plan != ksft_test_num()) printf("# Planned tests != run tests (%u != %u)\n", ksft_plan, ksft_test_num()); diff --git a/tools/testing/selftests/kselftest/ksft.py b/tools/testing/selftests/kselftest/ksft.py index bf215790a89d..7675a15a1264 100644 --- a/tools/testing/selftests/kselftest/ksft.py +++ b/tools/testing/selftests/kselftest/ksft.py @@ -27,6 +27,9 @@ def set_plan(num_tests): def print_cnts(): + if ksft_cnt['skip'] > 0: + print("# Skipped tests detected. Consider enabling relevant config options to improve coverage.") + print( f"# Totals: pass:{ksft_cnt['pass']} fail:{ksft_cnt['fail']} xfail:0 xpass:0 skip:{ksft_cnt['skip']} error:0" ) diff --git a/tools/testing/selftests/kselftest/ktap_helpers.sh b/tools/testing/selftests/kselftest/ktap_helpers.sh index 79a125eb24c2..a4211221ccd6 100644 --- a/tools/testing/selftests/kselftest/ktap_helpers.sh +++ b/tools/testing/selftests/kselftest/ktap_helpers.sh @@ -107,5 +107,9 @@ ktap_finished() { } ktap_print_totals() { + if [ "$KTAP_CNT_SKIP" -gt 0 ]; then + echo "# Skipped tests detected. " \ + "Consider enabling relevant config options to improve coverage." + fi echo "# Totals: pass:$KTAP_CNT_PASS fail:$KTAP_CNT_FAIL xfail:0 xpass:0 skip:$KTAP_CNT_SKIP error:0" }
Update the functions that print the test totals at the end of a selftest to include a warning message when skipped tests are detected. The message advises users that skipped tests may indicate missing configuration options and suggests enabling them to improve coverage. Signed-off-by: Laura Nao <laura.nao@collabora.com> --- This patch follows up on a previous discussion[1] and aims to highlight skipped tests for the user's attention. [1] https://lore.kernel.org/lkml/2bb2d338-cd00-4ac2-b8bd-5579eae82637@linuxfoundation.org/ --- tools/testing/selftests/kselftest.h | 4 ++++ tools/testing/selftests/kselftest/ksft.py | 3 +++ tools/testing/selftests/kselftest/ktap_helpers.sh | 4 ++++ 3 files changed, 11 insertions(+)