Re: Git Test Coverage Report for v2.37.0-rc0

[Taylor Blau <me@ttaylorr.com>]

On Tue, Jun 14, 2022 at 04:20:05PM -0400, Derrick Stolee wrote:
> > Taylor Blau	f9825d1c builtin/repack.c: support generating a cruft pack
> > builtin/repack.c
> > f9825d1c 680) strvec_pushf(&cmd.args, "--cruft-expiration=%s",
>
> The --cruft-expiration option is not tested.

Note that only the `--cruft-expiration` option of `git repack` is
untested. This code is just passing down the expiration from `repack`'s
perspective down to pack-objects, which has extensive coverage of the
`--cruft-expiration` option in t5329.

It does feel a little weird to leave this uncovered, but I don't think
it's substantially different from the uncovered lines in setting up
`cruft_po_args` on lines 86-90 of the repack code.

> > f9825d1c 708) fprintf(in, "%s.pack\n", item->string);
>
> This is related to the existence of .keep packs. A corner case, but maybe
> worth exploring.

Probably worth testing. We're ensuring that `.keep` packs in addition to
`--keep-pack`s are excluded from the cruft pack. That isn't tested and
probably should be.
>
> > pack-write.c
> > 5dfaf49a 330) unlink(mtimes_name);
> > 5dfaf49a 331) fd = xopen(mtimes_name, O_CREAT|O_EXCL|O_WRONLY, 0600);
>
> This appears to be an interesting case for the write_mtimes_file() method,
> since its first parameter is 'mtimes_name' and all tested cases are
> passing NULL, it seems.

Ugh, yes :-). That's imitating a common pattern in the write_xyz_file()
for xyz referring to auxiliary pack data. The cleanup should be pretty
straightforward, I'll send a patch or two shortly...

Thanks,
Taylor