Re: [PATCH v3 2/8] t/helper: add 'find-pack' test-tool

From: Christian Couder <christian.couder@gmail.com>
Date: Aug 8, 2023, 08:28 UTC
Message-ID: <CAP8UFD3_XOtY-cetnO6sPSj3N-EpF+eq836Vv0v8jPDTSkVupA@mail.gmail.com>
In-Reply-To: <ZMBQRRppsadqa8Ua@nand.local>

On Wed, Jul 26, 2023 at 12:44 AM Taylor Blau <me@ttaylorr.com> wrote:

Show 20 quoted lines

>
> On Mon, Jul 24, 2023 at 10:59:03AM +0200, Christian Couder wrote:
> > ---
> >  Makefile                  |  1 +
> >  t/helper/test-find-pack.c | 35 +++++++++++++++++++++++++++++++++++
> >  t/helper/test-tool.c      |  1 +
> >  t/helper/test-tool.h      |  1 +
> >  4 files changed, 38 insertions(+)
> >  create mode 100645 t/helper/test-find-pack.c
>
> Everything that you wrote here seems reasonable to me, and the
> implementation of the new test tool is very straightforward.
>
> I'm pretty sure that everything here is correct, and we'll implicitly
> test the behavior of the new helper in following patches.
>
> That said, I think that it might be prudent here to "test the tests" and
> write a simple test script that exercises this test helper over a more
> trivial case. There is definitely prior art for testing our helpers
> directly in the t00?? tests.

Ok, I have written a new t0080-find-pack.sh test script for this in the version 4 I just sent.

I have also changed `test-tool find-pack` so that it now accepts a `--check-count <n>` option. This addresses some of your comments on another patch in the previous version of this series. As the code is now a bit more complex, there is more justification for a test script.

Thanks.