Re: [PATCH v2 1/4] t: move reftable/readwrite_test.c to the unit testing framework

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

Chandra Pratap <chandrapratap3519@gmail.com> writes:

> reftable/readwrite_test.c exercises the functions defined in
> reftable/reader.{c,h} and reftable/writer.{c,h}. Migrate
> reftable/readwrite_test.c to the unit testing framework. Migration
> involves refactoring the tests to use the unit testing framework
> instead of reftable's test framework and renaming the tests to
> align with unit-tests' naming conventions.
>
> Since some tests in reftable/readwrite_test.c use the functions
> set_test_hash(), noop_flush() and strbuf_add_void() defined in
> reftable/test_framework.{c,h} but these files are not #included
> in the ported unit test, copy these functions in the new test file.
>
> While at it, ensure structs are 0-initialized with '= { 0 }'
> instead of '= { NULL }'.

OK.

> -		EXPECT(buf->buf[off] == 'r');
> +		if (!off)
> +			off = header_size((hash_id == GIT_SHA256_FORMAT_ID) ? 2 : 1);
> +		check(buf->buf[off] == 'r');

Why not "check_char(buf->buf[off], ==, 'r')"?

>  	}
>  
> -	EXPECT(stats->log_stats.blocks > 0);
> +	check(stats->log_stats.blocks > 0);

Why not "check_int(stats->log_stats.blocks, >, 0)", which you used
in the t_log_write_read() function?

While reading this step, I looked for use of check() that is not
rewriting EXPECT_ERR(x) to check(!x) as suspicious.  The above two
(and a !memcmp() that is OK) were the only three such uses of
check(), I think.

Thanks.