All of lore.kernel.org
 help / color / mirror / Atom feed
From: Derrick Stolee <derrickstolee@github.com>
To: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Cc: gitster@pobox.com, chakrabortyabhradeep79@gmail.com
Subject: Re: [PATCH] test-lib-functions: fix test_subcommand_inexact
Date: Tue, 22 Mar 2022 11:17:51 -0400	[thread overview]
Message-ID: <540936ba-7287-77fa-9cee-e257ed3c119d@github.com> (raw)
In-Reply-To: <pull.1185.git.1647894845421.gitgitgadget@gmail.com>

On 3/21/22 4:34 PM, Derrick Stolee via GitGitGadget wrote:
> 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.

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

  reply	other threads:[~2022-03-22 15:17 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-21 20:34 [PATCH] test-lib-functions: fix test_subcommand_inexact Derrick Stolee via GitGitGadget
2022-03-22 15:17 ` Derrick Stolee [this message]
2022-03-23 14:53   ` Junio C Hamano
2022-03-23 14:55     ` Derrick Stolee
2022-03-23 21:45       ` Taylor Blau
2022-03-23 23:10         ` Junio C Hamano
2022-03-24 15:42           ` Derrick Stolee
2022-03-24 16:02             ` Taylor Blau
2022-03-24 16:39               ` Derrick Stolee
2022-03-24 16:38             ` Junio C Hamano
2022-03-24 18:10     ` Abhradeep Chakraborty
2022-03-25  0:33       ` Junio C Hamano
2022-03-25  8:13         ` Abhradeep Chakraborty
2022-03-24 18:34 ` [PATCH v2 0/2] " Derrick Stolee via GitGitGadget
2022-03-24 18:34   ` [PATCH v2 1/2] t7700: check post-condition in kept-pack test Derrick Stolee via GitGitGadget
2022-03-24 18:58     ` Taylor Blau
2022-03-25 13:55       ` Derrick Stolee
2022-03-25 17:07     ` Junio C Hamano
2022-03-25 17:23       ` Derrick Stolee
2022-03-25 17:36         ` Taylor Blau
2022-03-25 18:22           ` Junio C Hamano
2022-03-24 18:34   ` [PATCH v2 2/2] test-lib-functions: fix test_subcommand_inexact Derrick Stolee via GitGitGadget
2022-03-24 18:49     ` Taylor Blau
2022-03-24 20:48     ` Junio C Hamano
2022-03-25 14:03       ` Derrick Stolee
2022-03-25 17:25         ` Junio C Hamano
2022-03-25 19:02   ` [PATCH v3 0/2] " Derrick Stolee via GitGitGadget
2022-03-25 19:02     ` [PATCH v3 1/2] t7700: check post-condition in kept-pack test Derrick Stolee via GitGitGadget
2022-03-25 19:02     ` [PATCH v3 2/2] test-lib-functions: remove test_subcommand_inexact Derrick Stolee via GitGitGadget
2022-03-30  2:44     ` [PATCH v3 0/2] test-lib-functions: fix test_subcommand_inexact Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=540936ba-7287-77fa-9cee-e257ed3c119d@github.com \
    --to=derrickstolee@github.com \
    --cc=chakrabortyabhradeep79@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.