On Mon, Oct 23, 2023 at 12:42:08PM -0400, Taylor Blau wrote:
> On Wed, Oct 18, 2023 at 07:35:37AM +0200, Patrick Steinhardt wrote:
> > In t4207 we set up a set of replace objects via git-replace(1). Because
> > these references should not be impacting subsequent tests we also set up
> > some cleanup logic that deletes the replacement references via a call to
> > `rm -rf`. This reaches into the internal implementation details of the
> > reference backend and will thus break when we grow an alternative refdb
> > implementation.
> >
> > Refactor the tests to delete the replacement refs via Git commands so
> > that we become independent of the actual refdb that's in use. As we
> > don't have a nice way to delete all replacements or all references in a
> > certain namespace, we opt for a combination of git-for-each-ref(1) and
> > git-update-ref(1)'s `--stdin` mode.
> >
> > Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> > ---
> > t/t4207-log-decoration-colors.sh | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
> > index 21986a866df..d138e513a04 100755
> > --- a/t/t4207-log-decoration-colors.sh
> > +++ b/t/t4207-log-decoration-colors.sh
> > @@ -71,7 +71,7 @@ ${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A
> > '
> >
> > test_expect_success 'test coloring with replace-objects' '
> > - test_when_finished rm -rf .git/refs/replace* &&
> > + test_when_finished "git for-each-ref refs/replace*/** --format=${SQ}delete %(refname)${SQ} | git update-ref --stdin" &&
>
> Here and below, should we avoid the for-each-ref showing up on the
> left-hand side of the pipe? I'd think we want something closer to:
>
> test_when_finished "git for-each-ref refs/replace*/** --format=${SQ}delete %(refname)${SQ} >in && git update-ref --stdin <in" &&
>
> But having to quote the --format argument with "${SQ}"s makes the whole
> thing a little awkward to read and parse.
>
> Do you think that something like the below would be a readability
> improvement?
Yes, this certainly looks like a good improvement to me, thanks!
Patrick
> diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
> index d138e513a0..de8f6638cb 100755
> --- a/t/t4207-log-decoration-colors.sh
> +++ b/t/t4207-log-decoration-colors.sh
> @@ -70,8 +70,13 @@ ${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A
> cmp_filtered_decorations
> '
>
> --- >8 ---
> +remove_replace_refs () {
> + git for-each-ref 'refs/replace*/**' --format='delete %(refname)' >in &&
> + git update-ref --stdin <in
> +}
> +
> test_expect_success 'test coloring with replace-objects' '
> - test_when_finished "git for-each-ref refs/replace*/** --format=${SQ}delete %(refname)${SQ} | git update-ref --stdin" &&
> + test_when_finished remove_replace_refs &&
> test_commit C &&
> test_commit D &&
>
> @@ -99,7 +104,7 @@ EOF
> '
>
> test_expect_success 'test coloring with grafted commit' '
> - test_when_finished "git for-each-ref refs/replace*/** --format=${SQ}delete %(refname)${SQ} | git update-ref --stdin" &&
> + test_when_finished remove_replace_refs &&
>
> git replace --graft HEAD HEAD~2 &&
> --- 8< ---
>
> Thanks,
> Taylor
Attachment:
signature.asc
Description: PGP signature
