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. Among the test helpers that I can think of off the top of my head, I think a good handful of them have tests: - t0011-hashmap.sh - t0015-hash.sh - t0016-oidmap.sh - t0019-json-writer.sh - t0052-simple-ipc.sh - t0060-path-utils.sh - t0061-run-command.sh - t0063-string-list.sh - t0064-oid-array.sh - t0066-dir-iterator.sh - t0095-bloom.sh I would definitely recommend adding a test here, too. Like I said earlier, I think that you are implicitly testing the new behavior here, but it's going to happen in much more complicated environments than something you could construct synthetically here. Thanks, Taylor
