On Fri, 9 Aug 2024 at 23:42, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Chandra Pratap <chandrapratap3519@xxxxxxxxx> 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')"?
I wrote this series quite some time ago when this functionality
was not yet introduced to the unit testing framework. I'll commit
this change in the next reroll.
> > }
> >
> > - 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?
Looks like a case of too-mechanical-a-translation to me. I'll fix this
in the next version.
> 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.
I went through this series again and I agree on not encountering
any other subpar translations of EXPECT() to its counterparts in
the unit testing framework. I'll reroll the series with only these
changes until someone else finds any other corrections.
