libcheck / check

A unit testing framework for C
GNU Lesser General Public License v2.1
1.07k stars 210 forks source link

Add extra NULL argument at the end of fail* APIs #298

Closed brarcher closed 4 years ago

brarcher commented 4 years ago

The fail, fail_unless, and fail_if APIs were expected to contain a message explaining the failure. This was never enforced, and it was possible to write unit tests without providing messages.

In #249 a change was introduced to add printf argument checking to the Check assert APIs, including the fail APIs. There were a few fixes for this in http://github.com/libcheck/check/releases/tag/0.15.1. Those changes proved problematic for the uses of the fail* APIs without arguments, as those uses were now flagged as missing the necessary arguments.

A fix proposed by heftig in http://github.com/libcheck/check/issues/293 is to add a new NULL to the end of every fail call in the macro itself. For users of these APIs who do pass a message there will be a new warning about too many arguments. As the fail APIs are deprecated, this new warning is a reasonable trade-off, and can be avoided by switching fail calls to ck_assert* calls.

If a fail* APIs is passing a message, the emitted warning might be:

check_check_sub.c:65:23: warning: too many arguments for format [-Wformat-extra-args]
   fail_unless(1 == 2, "This test should fail");
                       ^
../src/check.h:472:77: note: in definition of macro ‘fail_unless’
     _ck_assert_failed(__FILE__, __LINE__, "Assertion '"#expr"' failed" , ## __VA_ARGS__, NULL)
                                                                             ^~~~~~~~~~~
mikkoi commented 4 years ago

The commit is question is "Add macro CK_ATTRIBUTE_FORMAT" https://github.com/libcheck/check/pull/249/commits/5e2b32fccc8c4c2440fdf8eccd0eb73fc5a3dcd9

I wanted to use GCC's facilities (since they are available). But this fix is for all users, not just GCC users. It might react differently in different compilers.

But if that part of the API is deprecated anyway, it might work even if it might cause some extra headache.

Otherwise, just get rid of my commit "Add macro CK_ATTRIBUTE_FORMAT". It was wishful thinking on my part. :-)