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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 9, 2024, 18:12 UTC
Message-ID: <xmqqwmkpd0qs.fsf@gitster.g>
In-Reply-To: <20240809111312.4401-2-chandrapratap3519@gmail.com>

Chandra Pratap <chandrapratap3519@gmail.com> writes:

Show 14 quoted lines

> 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.