Re: [PATCH v3] t/unit-tests: convert hash to use clar test framework

[Junio C Hamano <gitster@pobox.com>]

Seyi Kuforiji <kuforiji98@gmail.com> writes:

> Adapt the hash test functions to clar framework by using clar
> assertions where necessary. Following the consensus to convert
> the unit-tests scripts found in the t/unit-tests folder to clar driven by
> Patrick Steinhardt. Test functions are structured as a standalone to
> test individual hash string and literal case.
>
> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>

The change to the test program was a very pleasant read.  It was
trivially obvious that the new version faithfully rewrites the
original.  E.g., ...

>  static void check_hash_data(const void *data, size_t data_length,
>  			    const char *expected_hashes[])
>  {
> -	if (!check(data != NULL)) {
> -		test_msg("BUG: NULL data pointer provided");
> -		return;
> -	}
> +	cl_assert(data != NULL);

... instead of using check() and giving message with test_msg(), the
clar framework gives cl_assert() for us to use.

And ...

>  #define TEST_HASH_STR(data, expected_sha1, expected_sha256) do { \
>  		const char *expected_hashes[] = { expected_sha1, expected_sha256 }; \
> -		TEST(check_hash_data(data, strlen(data), expected_hashes), \
> -		     "SHA1 and SHA256 (%s) works", #data); \
> +		check_hash_data(data, strlen(data), expected_hashes); \

... instead of TEST() macro with the title string, we call the
underlying test function.  The loss of the message does not hurt us,
as both the test suite name and name of each test are shown by the
clar framework.

> -int cmd_main(int argc UNUSED, const char **argv UNUSED)
> +void test_hash__empty_string(void)
>  {
> -	struct strbuf aaaaaaaaaa_100000 = STRBUF_INIT;
> -	struct strbuf alphabet_100000 = STRBUF_INIT;
> -
> -	strbuf_addstrings(&aaaaaaaaaa_100000, "aaaaaaaaaa", 100000);
> -	strbuf_addstrings(&alphabet_100000, "abcdefghijklmnopqrstuvwxyz", 100000);
> -
>  	TEST_HASH_STR("",
>  		"da39a3ee5e6b4b0d3255bfef95601890afd80709",
>  		"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855");
> +}

And the fact that the body of each test function are unchanged from
the original helps to build confidence in the faithfulness of the
conversion.

The strbuf allocation is lost from here, and clean-up is lost from
the end of the file, and they are done in the function that needs
the strbuf, which also contributes to the clarity of the new
version.

Nicely done.  Will queue.

Thanks.