[0/2] Generalize reference locking in tests

Message

Derrick Stolee via GitGitGadget Jan. 10, 2024, 6:52 p.m. UTC

There are two tests in t1401 and t5541 that rely on writing a reference lock
file directly. While this works for the files reference backend, reftable
reference locks operate differently and are incompatible with this approach.
To be reference backend agnostic, this patch series refactors the two tests
to use git-update-ref(1) to lock refs instead.

This approach is more verbose and may warrant consideration of implementing
an abstraction to further simplify this operation. There appears to be one
other existing test in t1400 that also follows this pattern. Being that
there is only a handful of affected tests the implemented approach may be
sufficient as is.

Justin Tobler (2):
  t1401: generalize reference locking
  t5541: generalize reference locking

 t/t1401-symbolic-ref.sh    | 28 ++++++++++++++++++++++++----
 t/t5541-http-push-smart.sh | 25 +++++++++++++++++++++++--
 2 files changed, 47 insertions(+), 6 deletions(-)


base-commit: 624eb90fa8f65a79396615f3c2842ac5a3743350
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1634%2Fjltobler%2Fjt%2Fbackend-generic-ref-lock-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1634/jltobler/jt/backend-generic-ref-lock-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1634

Comments

Junio C Hamano Jan. 11, 2024, 12:36 a.m. UTC | #1

"Justin Tobler via GitGitGadget" <[e-mail]> writes:

> This approach is more verbose and may warrant consideration of implementing
> an abstraction to further simplify this operation. There appears to be one
> other existing test in t1400 that also follows this pattern. Being that
> there is only a handful of affected tests the implemented approach may be
> sufficient as is.

The use of two fifos and avoiding deadlocking parent and child is
tricky enough that it does feel that it warrants a helper function
to do the common part of what these two patches add.

I think I read t1401 patch carefully enough to understand what is
going on, but I cannot yet claim the same for the other one.

Thanks.

> Justin Tobler (2):
>   t1401: generalize reference locking
>   t5541: generalize reference locking
>
>  t/t1401-symbolic-ref.sh    | 28 ++++++++++++++++++++++++----
>  t/t5541-http-push-smart.sh | 25 +++++++++++++++++++++++--
>  2 files changed, 47 insertions(+), 6 deletions(-)
>
>
> base-commit: 624eb90fa8f65a79396615f3c2842ac5a3743350
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1634%2Fjltobler%2Fjt%2Fbackend-generic-ref-lock-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1634/jltobler/jt/backend-generic-ref-lock-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1634

Justin Tobler Jan. 11, 2024, 8:20 p.m. UTC | #2

Junio C Hamano <[e-mail]> writes:

> The use of two fifos and avoiding deadlocking parent and child is
> tricky enough that it does feel that it warrants a helper function
> to do the common part of what these two patches add.

To avoid the trickiness that comes with introducing fifos to these
tests, in the next patch version I've followed the suggestion of Peff to
instead create d/f conflicts to create an error condition. It appears
that these tests are not particular to the exact error condition being
triggered and therefore d/f conflicts are much simpler to produce.

Thanks,
Justin

On Wed, Jan 10, 2024 at 6:36 PM Junio C Hamano <[e-mail]> wrote:
>
> "Justin Tobler via GitGitGadget" <[e-mail]> writes:
>
> > This approach is more verbose and may warrant consideration of implementing
> > an abstraction to further simplify this operation. There appears to be one
> > other existing test in t1400 that also follows this pattern. Being that
> > there is only a handful of affected tests the implemented approach may be
> > sufficient as is.
>
> The use of two fifos and avoiding deadlocking parent and child is
> tricky enough that it does feel that it warrants a helper function
> to do the common part of what these two patches add.
>
> I think I read t1401 patch carefully enough to understand what is
> going on, but I cannot yet claim the same for the other one.
>
> Thanks.
>
> > Justin Tobler (2):
> >   t1401: generalize reference locking
> >   t5541: generalize reference locking
> >
> >  t/t1401-symbolic-ref.sh    | 28 ++++++++++++++++++++++++----
> >  t/t5541-http-push-smart.sh | 25 +++++++++++++++++++++++--
> >  2 files changed, 47 insertions(+), 6 deletions(-)
> >
> >
> > base-commit: 624eb90fa8f65a79396615f3c2842ac5a3743350
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1634%2Fjltobler%2Fjt%2Fbackend-generic-ref-lock-v1
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1634/jltobler/jt/backend-generic-ref-lock-v1
> > Pull-Request: https://github.com/gitgitgadget/git/pull/1634