Re: [PATCH] test-lib-functions: fix test_subcommand_inexact

[Derrick Stolee <derrickstolee@github.com>]

On 3/23/2022 7:10 PM, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
> 
>> On Wed, Mar 23, 2022 at 10:55:37AM -0400, Derrick Stolee wrote:
>>>> So, perhaps #3 ;-)?
>>>
>>> I'll default to #3 (do nothing), but if this shows up again
>>> I'll plan on adding a comment to the helper to be clear on
>>> how "inexact" the helper really is.
>>
>> I wonder if we could sidestep the whole issue with
>> test_subcommand_inexact by testing this behavior by looking at the
>> contents of the packs themselves.
>>
>> If we have a kept pack, and then add some new objects, and run "git
>> repack --write-midx -adb", the new pack should not contain any of the
>> objects found in the old (kept) pack. And that's the case after this
>> patch, but was broken before it.
> 
> Sounds quite sensible.
> 
> Instead of saying "we are happy as long as we internally run this
> command, as that _should_ give us the desired outcome", we check the
> resulting packs ourselves, and we do not really care how the
> "repack" command gave us that desired outcome.

Sounds good. It's all about a balance: using test_subcommand[_inexact]
gives us a way to check "Did we trigger this other command that we
trust works correctly from other tests?" without the more complicated
work of doing a full post-condition check. It's a bit more of a unit-
level check than most Git tests.

The full post-condition check requires more test code, but that's not
really a problem. The problem comes in if that test is now too rigid
to future changes in that subcommand. What if the post-conditions
change in a subtle way because of the subcommand does something
differently, but in a way that is not of importance to the top
command?

In this specific case, the test name says that it "packs non-kept
objects", so we can do more here to validate that post-condition
that we care about.

As I'm looking at Taylor's test case example, the one thing I notice
is that there is only one pack-file before the repack. It would be
good to have a non-kept packfile get repacked in the process, not
just the loose objects added by the test_commit. I'll take a look at
what can be done here.

Thanks,
-Stolee