patchtest-lib-functions: fix test_subcommand_inexact

From: Derrick Stolee <derrickstolee@github.com>

The implementation of test_subcommand_inexact() was originally introduced in e4d0c11c0 (repack: respect kept objects with '--write-midx -b', 2021-12-20) with the intention to allow finding a subcommand based on an initial set of arguments. The inexactness was intended as a way to allow flexible options beyond that initial set, as opposed to test_subcommand() which requires that the full list of options is provided in its entirety.

The implementation began by copying test_subcommand() and replaced the repeated argument 'printf' statement to append ".*" instead of "," to each argument. This has a few drawbacks:

1. Most importantly, this repeats the use of ".*" within 'expr', so the
   inexact match is even more flexible than expected. It allows the list
   of arguments to exist as a subsequence (with any other items included
   between those arguments).

2. The line 'expr="$(expr%,}"' that previously removed a trailing comma
   now no longer does anything, since the string ends with ".*".

Both of these issues are fixed by keeping the addition of the comma in the printf statement, then adding ".*" after stripping out the trailing comma.

All existing tests continue to pass with this change, since none of them were taking advantage of this unintended flexibility.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
    test-lib-functions: fix test_subcommand_inexact
    
    This fixes a concern discovered by Junio in [1].
    
    [1] https://lore.kernel.org/git/xmqq4k41vdwe.fsf@gitster.g/
    
    Thanks, -Stolee

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1185%2Fderrickstolee%2Ftest-subcommand-fix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1185/derrickstolee/test-subcommand-fix-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1185

 t/test-lib-functions.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

On 3/21/22 4:34 PM, Derrick Stolee via GitGitGadget wrote:

For some reason, I thought I had run all tests on my machine to passing, but I was wrong.

Instead, t7700-repack.sh fails because of this test:

test_expect_success '--write-midx -b packs non-kept objects' '
	GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
		git repack --write-midx -a -b &&
	test_subcommand_inexact git pack-objects --honor-pack-keep <trace.txt
'

Specifically, "git pack-objects" has several options before "--honor-pack-keep" including the temporary pack name and a "--keep-true-parents" flag.

So, this patch is incorrect about keeping things working. The options are:

1. Keep the repeated ".*" and be clear about the expectations.
   (This could drop the "remove trailing comma" line.)

2. Find another way to test this --write-midx behavior while
   keeping the stricter test_subcommand_inexact helper.

3. Something else???

Thanks, -Stolee

Derrick Stolee <derrickstolee@github.com> writes:

The result of doing #1 is still "inexact" but at that point it is unclear if we are being way too inexact to be useful. If the looseness bothers us too much, we may decide that #1 is not worth doing. But obviously the looseness did not bother us that much until last week, so probably an obvious #3, do nothing, letting the sleeping dog lie, might be what we want to do?

If we were to pursue #2, then, would we tightening the test for the write-midx using the "stricter" helper, or would the stricter one be too strict to be useful for that case? If we are rewriting the write-midx test by not using the "stricter" helper, then we would be creating a stricter one nobody uses, which sounds quite wasteful.

It seems that the only case that could result in a result that is better than "do nothing" is if we can use a different pattern with the "stricter" helper to express what "write-midx" test wanted to do, but because what we need to fuzzily match on the command line in that test includes a generated temporary filename, I do not think it is likely to be easily doable.

So, perhaps #3 ;-)?

On 3/23/2022 10:53 AM, Junio C Hamano wrote:

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.

Thanks, -Stolee

On Wed, Mar 23, 2022 at 10:55:37AM -0400, Derrick Stolee wrote:

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.

Here's a test which constructs that scenario and then asserts that there isn't any overlap between the newly created pack and the old, kept pack.

--- 8< ---

Taylor Blau <me@ttaylorr.com> writes:

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.

On 3/23/2022 7:10 PM, Junio C Hamano wrote:

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

On Thu, Mar 24, 2022 at 11:42:44AM -0400, Derrick Stolee wrote:

I think you are too good at nerd-sniping me ;-). Here's a more robust test, that I think reads a little cleaner than the previous round. Let me know what you think:

--- 8< ---

On 3/24/2022 12:02 PM, Taylor Blau wrote:

Finally, you found my most redeeming quality!

This is an interesting way to get this set of objects without storing the original pack name. It might be good to keep consistent with the way we get the new objects, though.

I've got a modification of your original design prepared in my GGG PR and will send a v2 including it after it passes all of the builds.

Thanks, -Stolee

Derrick Stolee <derrickstolee@github.com> writes:

Thanks for clearly laying out the way to think about the issue. I agree with all of it, of course ;-)

Junio C Hamano <gitster@pobox.com> wrote:

Personally, I would prefer #3 i.e. do nothing (even in the future; unless it is removed all together). I also think that the current behaviour is not "too inexact". Rather it would be too strict for `test_subcommand_inexact` if we remove the ".*" thing here.

Inexact means that the line needs not to be exactly same - there may be some words in between the desired words (in this case, any flags that come between the desired sub-commands). The current behaviour (i.e. `local expr=$(printf '"%s".*' "$@")`) is justifying the name of the function. Replacing ".*" with "," will therefore not work as the name of the function suggests - it will rather work as `test_subcommand_starts_with`.

Only the `expr=${expr%,}.*" line needs to be changed, I think.

Thanks :)

Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> writes:

The current name may justify what it does, but so does "declare match randomly" would. How much fuzziness we tolerate is something that must be made clear to help developers who may potentially want to use it, and the phrase "inexact" alone would not help us.

Junio C Hamano <gitster@pobox.com> wrote:

Yep! I agree.

Thanks :)

Junio discovered in [1] that test_subcommand_inexact is more flexible than initially intended.

[1] https://lore.kernel.org/git/xmqq4k41vdwe.fsf@gitster.g/

The intention was that we do not need to specify the remaining arguments for a subcommand, but instead the current behavior is to allow the given arguments to appear as any subsequence within the command (except that the first "git" instance must be the first argument).

This changes the helper to be more rigid.

Changes in v2 =============

 * After noticing that t7700 fails with the helper change, Taylor wrote a
   modification of the test to check the post-conditions directly and avoid
   the helper. This is included as Patch 1 with some modifications to be
   easier to read.

 * Patch 2 is the v1 patch with some change to the commit message about why
   the tests pass after patch 1.

Thanks, -Stolee

Derrick Stolee (2):
  t7700: check post-condition in kept-pack test
  test-lib-functions: fix test_subcommand_inexact

 t/t7700-repack.sh       | 52 ++++++++++++++++++++++++++++++++++++++---
 t/test-lib-functions.sh |  4 ++--
 2 files changed, 51 insertions(+), 5 deletions(-)

base-commit: a68dfadae5e95c7f255cf38c9efdcbc2e36d1931
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1185%2Fderrickstolee%2Ftest-subcommand-fix-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1185/derrickstolee/test-subcommand-fix-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1185

Range-diff vs v1:

 -:  ----------- > 1:  f2f8d12929b t7700: check post-condition in kept-pack test
 1:  50176722bbf ! 2:  ed67b748971 test-lib-functions: fix test_subcommand_inexact
     @@ Commit message
          the printf statement, then adding ".*" after stripping out the trailing
          comma.
      
     -    All existing tests continue to pass with this change, since none of them
     -    were taking advantage of this unintended flexibility.
     +    All existing tests continue to pass with this change. There was one
     +    instance from t7700-repack.sh that was taking advantage of this
     +    flexibility, but it was removed in the previous change.
      
     +    Helped-by: Taylor Blau <me@ttaylorr.com>
          Signed-off-by: Derrick Stolee <derrickstolee@github.com>
      
       ## t/test-lib-functions.sh ##

From: Derrick Stolee <derrickstolee@github.com>

The '--write-midx -b packs non-kept objects' test in t7700-repack.sh uses test_subcommand_inexact to check that 'git repack' properly adds the '--honor-pack-keep' flag to the 'git pack-objects' subcommand. However, the test_subcommand_inexact helper is more flexible than initially designed, and this instance is the only one that makes use of it: there are additional arguments between 'git pack-objects' and the '--honor-pack-keep' flag. In order to make test_subcommand_inexact more strict, we need to fix this instance.

This test checks that 'git repack --write-midx -a -b -d' will create a new pack-file that does not contain the objects within the kept pack. This behavior is possible because of the multi-pack-index bitmap that will bitmap objects against multiple packs. Without --write-midx, the objects in the kept pack would be duplicated so the resulting pack is closed under reachability and bitmaps can be created against it. This is discussed in more detail in e4d0c11c0 (repack: respect kept objects with '--write-midx -b', 2021-12-20) which also introduced this instance of test_subcommand_inexact.

To better verify the intended post-conditions while also removing this instance of test_subcommand_inexact, rewrite the test to check the list of packed objects in the kept pack and the list of the objects in the newly-repacked pack-file _other_ than the kept pack. These lists should be disjoint.

Be sure to include a non-kept pack-file and loose objects to be extra careful that this is properly behaving with kept packs and not just avoiding repacking all pack-files.

Co-authored-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 t/t7700-repack.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 49 insertions(+), 3 deletions(-)

On Thu, Mar 24, 2022 at 06:34:56PM +0000, Derrick Stolee via GitGitGadget wrote:

Nicely put.

And to be clear, I am totally OK with this usage of my signed-off-by.

It seems a little odd to me to pass the name of a single file as input to get_sorted_objects_from_packs over stdin. I probably would have expected something like `git show-index <"$1" >raw && ...` instead.

We may also want to s/packs/pack, since this function only will handle one index at a time.

Having the sort in there is my fault, but after reading this more carefully it's definitely unnecessary, since show-index will give us the results in lexical order by object name already.

I thought that here it might be easier to say:

    before="$(find $objdir/pack -name "*.idx")"

...and then replace "$(cat before)" with "$before", along with the other uses of the before file below. But it gets a little funny when you want to discover which is the new pack, where it is more natural to dump the output of comm into a file.

You could write "new_pack=$(comm -13 before after)", but debugging this test would be difficult if the output of comm there contained more than one line.

Though we probably want to check that we only get one line anyway here, since get_sorted_objects_from_packs will barf if we had more than one line in file new-pack here, too.

Thanks, Taylor

On 3/24/2022 2:58 PM, Taylor Blau wrote:

Based on the way we are creating a file whose contents is the name of the .idx file, we would at least use '$(cat "$1")'. I kind of like the symmetry of the input/output redirection when using the helper, but I can easily change this.

Yes.

Cool. Will drop.

For this reason, I'll continue to store the .idx names in files.

Thanks. Easy to add a test_line_count before this check.

-Stolee

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

As pointed out by Taylor, this "the standard input gives us the name of a file to be read" looks strange. It may work, and it may even give the easiest interface to use by all the callers, but if we were designing a more generic helper function suitable to be added to the test-lib*.sh, we wouldn't design it like so---instead it would be either "we read the contents of the .idx file from the standard input" or "the first argument is the name of the .idx file".

We probably want to sanity check "repack -a" by insisting "before" has found exactly one .idx file, before using it this way.

		test_line_count = 1 before &&
		before=$(cat before) &&
		>"${before%.idx}.keep"

OK.  "after" gets sorted because we will pass it to "comm" later.

Since we've made sure "before" is a one-liner earlier, we could just say

		test_cmp before kept &&

instead, no?

		
OK.

OK. This should give only one line of output but that is merely assumed. We know after has 2 and before has 1, but haven't made sure that before is a subset of after.

		test_line_count = 1 new-pack &&

OK.

Great.

On 3/25/2022 1:07 PM, Junio C Hamano wrote:

Ok. Can do.
 

Good idea. This mixture of a file and variable sharing a name is a bit muddy for me, though. Using "before_name" as the variable would be enough of a differentiator.

'before' contains a .idx name and 'kept' contains a .keep name, so this direct comparison does not work. We could do that additional check like this:

	kept_name=$(cat kept) &&
	echo ${kept_name%.keep}.idx >kept-idx &&
	test_cmp before kept-idx &&

Thanks for taking the time to clean this up, as it might become a good example for these kinds of post-condition checks of the packs directory in the future.

Thanks, -Stolee

On Fri, Mar 25, 2022 at 01:23:27PM -0400, Derrick Stolee wrote:

I think keeping this kind of post-condition check pretty minimal is favorable, since this is functionality of `git repack` (i.e., that `-a` leaves you with one kept) that is already tested thoroughly elsewhere.

So I'd probably avoid checking the output altogether, but if we did want to test it, I think something as quick and cheap as:

    test_line_count = 1 before

would do the trick.

Thanks, Taylor

Taylor Blau <me@ttaylorr.com> writes:

It is nice in principle, but for this particular script, whose helper function's implementation relies on these "assumptions" to work correctly (e.g. imagine what happens when 'before' file had 0 or 2 lines in it and you called the get_sorted_objects_from_packs helper), we should be more defensive, and that is where all my suggestions in the thread come from.

From: Derrick Stolee <derrickstolee@github.com>

The implementation of test_subcommand_inexact() was originally introduced in e4d0c11c0 (repack: respect kept objects with '--write-midx -b', 2021-12-20) with the intention to allow finding a subcommand based on an initial set of arguments. The inexactness was intended as a way to allow flexible options beyond that initial set, as opposed to test_subcommand() which requires that the full list of options is provided in its entirety.

The implementation began by copying test_subcommand() and replaced the repeated argument 'printf' statement to append ".*" instead of "," to each argument. This has a few drawbacks:

1. Most importantly, this repeats the use of ".*" within 'expr', so the
   inexact match is even more flexible than expected. It allows the list
   of arguments to exist as a subsequence (with any other items included
   between those arguments).

2. The line 'expr="$(expr%,}"' that previously removed a trailing comma
   now no longer does anything, since the string ends with ".*".

Both of these issues are fixed by keeping the addition of the comma in the printf statement, then adding ".*" after stripping out the trailing comma.

All existing tests continue to pass with this change. There was one instance from t7700-repack.sh that was taking advantage of this flexibility, but it was removed in the previous change.

Helped-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 t/test-lib-functions.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

On Thu, Mar 24, 2022 at 06:34:57PM +0000, Derrick Stolee via GitGitGadget wrote:

In my tree, the test modified by the previous patch was the only caller of `test_subcommand_inexact()`. Looking through the output of:

    git for-each-ref refs/remotes/origin/ --format='%(refname)' | xargs -L1 \
      git -P grep -l test_subcommand_inexact -- t | sort -u

it doesn't look like there are any other topics in flight [1] that call test_subcommand_inexact(), either.

Unless you have any pending series which want to call test_subcommand_inexact, I'd be just as happy to get rid of the function entirely.

Thanks, Taylor

[1]: My `origin` points to Junio's tree, so I'm looking through all of the topic branches before integration, not just the standard "master", "next", etc.

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

Of course all existing tests continue to pass, as we no longer have any user of test_subcommand_inexact after the previous step ;-).

Among

 (1) doing nothing,
 (2) removing, and
 (3) clarifying the implementation,

my preference would be 2 > 1 > 3.  If we add

 (4) clarify the implementation and document what kind of inexactness we
     tolerate with an updated comment"

to the mix, that would come before all 3 others, though.

Perhaps squash something like this in?

 t/test-lib-functions.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

On 3/24/2022 4:48 PM, Junio C Hamano wrote:

Yeah, I definitely should have checked to see if there were other uses of this. I thought there was, but I was mistaken.

I agree that (2) is the best option here.
 

Is there value in fixing the implementation and adding this comment
if we are to just delete the helper? I suppose that we could prevent
a future contribution from reintroducing the broken implementation.
 

This is an accurate description of what the fixed implementation does.

My current feeling is that we should just delete this and refer to that deletion if anyone considers needing something like it.

Thanks, -Stolee

Derrick Stolee <derrickstolee@github.com> writes:

That is a good thoguth to take into account.

I am very much in favor of deleting it.

Thanks.

Junio discovered in [1] that test_subcommand_inexact is more flexible than initially intended.

[1] https://lore.kernel.org/git/xmqq4k41vdwe.fsf@gitster.g/

The intention was that we do not need to specify the remaining arguments for a subcommand, but instead the current behavior is to allow the given arguments to appear as any subsequence within the command (except that the first "git" instance must be the first argument).

By changing the test that needed the helper, we can avoid the helper in the first place. Modify the test and remove the helper.

Changes in v3 =============

 * Significant edits to the test in t7700 based on Junio and Taylor's
   feedback.

 * Patch 2 now deletes the helper as it is not used anywhere.

Thanks, -Stolee

Derrick Stolee (2):
  t7700: check post-condition in kept-pack test
  test-lib-functions: remove test_subcommand_inexact

 t/t7700-repack.sh       | 57 ++++++++++++++++++++++++++++++++++++++---
 t/test-lib-functions.sh | 34 ------------------------
 2 files changed, 54 insertions(+), 37 deletions(-)

base-commit: a68dfadae5e95c7f255cf38c9efdcbc2e36d1931
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1185%2Fderrickstolee%2Ftest-subcommand-fix-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1185/derrickstolee/test-subcommand-fix-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1185

Range-diff vs v2:

 1:  f2f8d12929b ! 1:  fb2c550512e t7700: check post-condition in kept-pack test
     @@ t/t7700-repack.sh: test_expect_success '--write-midx with preferred bitmap tips'
       	)
       '
       
     -+get_sorted_objects_from_packs () {
     -+	git show-index <$(cat) >raw &&
     -+	cut -d" " -f2 raw | sort
     ++# The first argument is expected to be a filename
     ++# and that file should contain the name of a .idx
     ++# file. Send the list of objects in that .idx file
     ++# into stdout.
     ++get_sorted_objects_from_pack () {
     ++	git show-index <$(cat "$1") >raw &&
     ++	cut -d" " -f2 raw
      +}
      +
       test_expect_success '--write-midx -b packs non-kept objects' '
     @@ t/t7700-repack.sh: test_expect_success '--write-midx with preferred bitmap tips'
      +		test_commit base &&
      +		git repack -ad &&
      +		find $objdir/pack -name "*.idx" >before &&
     -+		>$objdir/pack/$(basename $(cat before) .idx).keep &&
     ++		test_line_count = 1 before &&
     ++		before_name=$(cat before) &&
     ++		>${before_name%.idx}.keep &&
      +
      +		# Create a non-kept pack-file
      +		test_commit other &&
     @@ t/t7700-repack.sh: test_expect_success '--write-midx with preferred bitmap tips'
      +		find $objdir/pack -name "*.idx" | sort >after &&
      +		test_line_count = 2 after &&
      +		find $objdir/pack -name "*.keep" >kept &&
     -+		test_line_count = 1 kept &&
     ++		kept_name=$(cat kept) &&
     ++		echo ${kept_name%.keep}.idx >kept-idx &&
     ++		test_cmp before kept-idx &&
      +
      +		# Get object list from the kept pack.
     -+		get_sorted_objects_from_packs \
     -+			<before \
     -+			>old.objects &&
     ++		get_sorted_objects_from_pack before >old.objects &&
      +
      +		# Get object list from the one non-kept pack-file
      +		comm -13 before after >new-pack &&
     -+		get_sorted_objects_from_packs \
     -+			<new-pack \
     -+			>new.objects &&
     ++		test_line_count = 1 new-pack &&
     ++		get_sorted_objects_from_pack new-pack >new.objects &&
      +
      +		# None of the objects in the new pack should
      +		# exist within the kept pack.
 2:  ed67b748971 < -:  ----------- test-lib-functions: fix test_subcommand_inexact
 -:  ----------- > 2:  f5a96a121a5 test-lib-functions: remove test_subcommand_inexact

From: Derrick Stolee <derrickstolee@github.com>

The '--write-midx -b packs non-kept objects' test in t7700-repack.sh uses test_subcommand_inexact to check that 'git repack' properly adds the '--honor-pack-keep' flag to the 'git pack-objects' subcommand. However, the test_subcommand_inexact helper is more flexible than initially designed, and this instance is the only one that makes use of it: there are additional arguments between 'git pack-objects' and the '--honor-pack-keep' flag. In order to make test_subcommand_inexact more strict, we need to fix this instance.

This test checks that 'git repack --write-midx -a -b -d' will create a new pack-file that does not contain the objects within the kept pack. This behavior is possible because of the multi-pack-index bitmap that will bitmap objects against multiple packs. Without --write-midx, the objects in the kept pack would be duplicated so the resulting pack is closed under reachability and bitmaps can be created against it. This is discussed in more detail in e4d0c11c0 (repack: respect kept objects with '--write-midx -b', 2021-12-20) which also introduced this instance of test_subcommand_inexact.

To better verify the intended post-conditions while also removing this instance of test_subcommand_inexact, rewrite the test to check the list of packed objects in the kept pack and the list of the objects in the newly-repacked pack-file _other_ than the kept pack. These lists should be disjoint.

Be sure to include a non-kept pack-file and loose objects to be extra careful that this is properly behaving with kept packs and not just avoiding repacking all pack-files.

Co-authored-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 t/t7700-repack.sh | 57 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 54 insertions(+), 3 deletions(-)

From: Derrick Stolee <derrickstolee@github.com>

The implementation of test_subcommand_inexact() was originally introduced in e4d0c11c0 (repack: respect kept objects with '--write-midx -b', 2021-12-20) with the intention to allow finding a subcommand based on an initial set of arguments. The inexactness was intended as a way to allow flexible options beyond that initial set, as opposed to test_subcommand() which requires that the full list of options is provided in its entirety.

The implementation began by copying test_subcommand() and replaced the repeated argument 'printf' statement to append ".*" instead of "," to each argument. This caused it to be more flexible than initially intended.

The previous change deleted the only use of test_subcommand_inexact, so instead of editing the helper, delete it.

Helped-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 t/test-lib-functions.sh | 34 ----------------------------------
 1 file changed, 34 deletions(-)

On Fri, Mar 25, 2022 at 07:02:45PM +0000, Derrick Stolee via GitGitGadget wrote:

I reviewed both patches carefully, and this version looks great to me. Thanks for all of your patience in cleaning up that sketch of the new test; you version in v3 here is delightfully easy to follow.

    Reviewed-by: Taylor Blau <me@ttaylorr.com>

Thanks, Taylor