Hi Shuah, On 7/23/24 18:17, Shuah Khan wrote: > On 7/22/24 09:43, Laura Nao wrote: >> Consider skipped tests in addition to passed tests when evaluating the >> overall result of the test suite in the finished() helper. >> >> Signed-off-by: Laura Nao <laura....@collabora.com> >> --- >> tools/testing/selftests/kselftest/ksft.py | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tools/testing/selftests/kselftest/ksft.py >> b/tools/testing/selftests/kselftest/ksft.py >> index cd89fb2bc10e..bf215790a89d 100644 >> --- a/tools/testing/selftests/kselftest/ksft.py >> +++ b/tools/testing/selftests/kselftest/ksft.py >> @@ -70,7 +70,7 @@ def test_result(condition, description=""): >> def finished(): >> - if ksft_cnt["pass"] == ksft_num_tests: >> + if ksft_cnt["pass"] + ksft_cnt["skip"] == ksft_num_tests: >> exit_code = KSFT_PASS > > Laura and Nícolas, > > I saw both your emails explaining how this fixes the problem in > a previous patch. > > However looks like you haven't see my response about the implications > of the exit_code = KSFT_PASS when tests are skipped. > > if ksft_cnt["pass"] + ksft_cnt["skip"] == ksft_num_tests: >> exit_code = KSFT_PASS > > Let me reiterate in case you missed it: > > There is a reason why you don't want to mark all tests passed > when there are several skips.Skips are an indication that > there are several tests and/or test cases that couldn't not > be run because of unmet dependencies. This condition needs > to be investigated to see if there are any config options > that could be enabled to get a better coverage. > > Including skips to determine pass gives a false sense security > that all is well when it isn't. > > So it is incorrect to set the exit code to KSFT_PASS when there > are skipped tests.
Just to be clear, the logic in ksft_finished() in kselftest.h (the C helper) is to exit with KSFT_PASS when there are only passed and skipped tests. You're suggesting we change it to exit with KSFT_FAIL in that case? Under this new logic, the runner would effectively treat skips as failures, impacting existing kselftests. > + if ksft_cnt["pass"] + ksft_cnt["skip"] == ksft_num_tests: > > >> else: >> exit_code = KSFT_FAIL > > The logic here seems to not take into account when you have a > conditions where you have a mixed results of passed tests, > skipped tests, and failed tests. > > Please revisit and figure out how to address this and report > the status correctly. The logic ensures that if there’s a mix of passed, skipped, and failed tests, the exit code returned is KSFT_FAIL. I believe that if there is at least one failed test, the overall test should be reported as failed, which is what happens in this case. Thanks, Laura
