RFC: rework assert_return() macro

[evelikov]

From the commit introducing it:

    The current assert_return() macro has a few short comings:
     - the name implies assert()
     - all users unnecessarily provide EXIT_FAILURE
     - the users cannot provide a message - "Failed assertion:" isn't great

    Tweak it up and in the process bump it to ALL_CAPS, to better indicate
    that it's a macro and not a function.

... and the follow-up commit:

    Swap the manual checks for the new OK() macro. In the process, we remove
    the cleanup in the error path.

    Many other tests don't bother, plus in the places that do try they don't
    always get it right/fully.

Before going forward and porting more tests, would be great to hear if the direction is right.

If* it is, a couple of questions:

• would people prefer different name - OK one is based on wine, but I'm not married to it ;-)
• how to handle the conversion - one test at a time, bulk (all assert_return -> OK, all manual -> OK), other?

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 65 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
testsuite/test-array.c	0.00%	0 Missing and 35 partials ⚠️
testsuite/test-hash.c	0.00%	0 Missing and 24 partials ⚠️
testsuite/test-blacklist.c	0.00%	0 Missing and 6 partials ⚠️

Files with missing lines	Coverage Δ
testsuite/test-blacklist.c	75.86% <0.00%> (ø)
testsuite/test-hash.c	84.07% <0.00%> (ø)
testsuite/test-array.c	70.00% <0.00%> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[lucasdemarchi]

The current assert_return() macro has a few short comings:
- the name implies assert()

• all users unnecessarily provide EXIT_FAILURE
• the users cannot provide a message - "Failed assertion:" isn't great

I think not needing to provide the same EXIT_FAILURE and being able to provide a message is good. The rename I think is not really needed. What's the problem with the term assert? IMO it's clearer than "ok". igt for example uses a bunch of igt_assert_*. ok() IMO is odd, but it maybe because I've never seen it used like this. Rust also has both Ok() and assert!(), which I think will add to the confusion.

how to handle the conversion - one test at a time, bulk (all assert_return -> OK, all manual -> OK), other?

I think a bulk one. If the message is optional, then the first patch is automated. Additional patches may follow later (maybe per compile unit) to add some good messages.

[evelikov]

What's the problem with the term assert?

There is nothing wrong with the term in itself. The snafu happens due to assert(3) and the associated NDEBUG, et al... As previously #239 (review)

Other naming options:

• add a prefix - ts_ (short for testsuite, after TS_EXPORT), other
• other names - check(), test()
• combination - eg. like igt ts_assert_ok or otherwise

Don't have a strong preference though.

I think a bulk one. If the message is optional, then the first patch is automated.

Awesome, will do.

Additional patches may follow later (maybe per compile unit) to add some good messages.

Do the example commits reach that bar, or shall I just drop them?