On Thu, Oct 17, 2024 at 11:29 AM Lorenzo Stoakes lorenzo.stoakes@oracle.com wrote:
On Thu, Oct 17, 2024 at 11:14:20AM -0700, Jeff Xu wrote:
Hi Lorenzo and Muhammad
Reviving this thread since the merging window is closed and we have more time to review /work on this code in the next few weeks.
On Fri, Sep 13, 2024 at 3:50 PM Jeff Xu jeffxu@chromium.org wrote:
Hi Lorenzo
On Sat, Sep 7, 2024 at 12:28 PM Lorenzo Stoakes lorenzo.stoakes@oracle.com wrote:
I also suggest we figure out this FAIL_TEST_IF_FALSE() thing at this point too - I may be missing something, but I cannot for the life me understand why we have to assert negations only, and other self tests do not do this.
My most test-infra related comments comes from Muhammad Usama Anjum (added into this email), e.g. assert is not recommended.[1] ,
[1] https://lore.kernel.org/all/148fc789-3c03-4490-a653-6a4e58f336b6@collabora.c...
Specifically regarding Lorenzo's comments about FAIL_TEST_IF_FALSE
Muhammad Usama Anjum doesn't want assert being used in selftest (see [1] above), and I quote: "We don't want to terminate the test if one test fails because of assert. We want the sub-tests to get executed in-dependent of other tests.
ksft_test_result(condition, fmt, ...); ksft_test_result_pass(fmt, ...);"
FAIL_TEST_IF_FALSE is a wrapper for ksft_test_result macro, and replacement of assert.
Please let me know if you have questions on this and Muhammad might also help to clarify the requirement if needed.
Thanks -Jeff
Right this is about not failing the test i.e. equivalent of an expect rather than an assert, which makes sense.
What I'm saying is we should have something more like
EXPECT_TRUE() EXPECT_FALSE()
etc.
Which would avoid these confusing
FAIL_TEST_IF_FALSE(!expr)
FAIL_TEST_IF_FALSE(expr) is the right way to use this macro.
It is same syntax as assert(expr), e.g:
man assert(expr) assert - abort the program if assertion is false
FAIL_TEST_IF_FALSE is a replacement for assert, instead of aborting the program, it just reports failure in this test.
Is this still confusing ? (The FAIL_TEST_IF_FALSE is already a descriptive name, and the syntax of assert is well known.)
Things.
Hopefully that's clear? Thanks!