On Thu, Oct 17, 2024 at 11:29 AM Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> 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@xxxxxxxxxxxx> wrote: > > > > > > Hi Lorenzo > > > > > > On Sat, Sep 7, 2024 at 12:28 PM Lorenzo Stoakes > > > <lorenzo.stoakes@xxxxxxxxxx> 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@xxxxxxxxxxxxx/ > > > > > 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!
