On Fri, Jan 07, 2022 at 05:31:04PM -0800, Junio C Hamano wrote: > Patrick Steinhardt <ps@xxxxxx> writes: > > > Add tests which demonstate which demonstrates that we're executing the > > You demonstrate too often, which may be the point of the test, but > looks wrong. > > I actually think this should be done as part of the fix to the code > itself, which presumably is a single-liner to tell the "skip when > running delete in packed-refs backend". IOW, just fix the code and > test how the externally observable behaviour of the code should be > in new tests, in the same commit. The reason why I chose to split this out into a separate commit is that it makes it easier to see what behaviour exactly is changing. If it was a single step, then a reader would only see the post-image behaviour but cannot reason about the pre-image behaviour without puzzling everything together. So personally I'd prefer to keep it as a separate step, but I'm not opposed to merging them if you still disagree with my reasoning above. Patrick > > diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh > > index 6c941027a8..0567fbdf0b 100755 > > --- a/t/t1416-ref-transaction-hooks.sh > > +++ b/t/t1416-ref-transaction-hooks.sh > > @@ -136,4 +136,68 @@ test_expect_success 'interleaving hook calls succeed' ' > > test_cmp expect target-repo.git/actual > > ' > > > > +test_expect_success 'hook does not get called on packing refs' ' > > + # Pack references first such that we are in a known state. > > + git pack-refs --all && > > + > > + write_script .git/hooks/reference-transaction <<-\EOF && > > + echo "$@" >>actual > > + cat >>actual > > + EOF > > + rm -f actual && > > + > > + git update-ref refs/heads/unpacked-ref $POST_OID && > > + git pack-refs --all && > > + > > + # We only expect a single hook invocation, which is the call to > > + # git-update-ref(1). But currently, packing refs will also trigger the > > + # hook. > > + cat >expect <<-EOF && > > + prepared > > + $ZERO_OID $POST_OID refs/heads/unpacked-ref > > + committed > > + $ZERO_OID $POST_OID refs/heads/unpacked-ref > > + prepared > > + $ZERO_OID $POST_OID refs/heads/unpacked-ref > > + committed > > + $ZERO_OID $POST_OID refs/heads/unpacked-ref > > + prepared > > + $POST_OID $ZERO_OID refs/heads/unpacked-ref > > + committed > > + $POST_OID $ZERO_OID refs/heads/unpacked-ref > > + EOF > > + > > + test_cmp expect actual > > +' > > + > > +test_expect_success 'deleting packed ref calls hook once' ' > > + # Create a reference and pack it. > > + git update-ref refs/heads/to-be-deleted $POST_OID && > > + git pack-refs --all && > > + > > + write_script .git/hooks/reference-transaction <<-\EOF && > > + echo "$@" >>actual > > + cat >>actual > > + EOF > > + rm -f actual && > > + > > + git update-ref -d refs/heads/to-be-deleted $POST_OID && > > + > > + # We only expect a single hook invocation, which is the logical > > + # deletion. But currently, we see two interleaving transactions, once > > + # for deleting the loose refs and once for deleting the packed ref. > > + cat >expect <<-EOF && > > + prepared > > + $ZERO_OID $ZERO_OID refs/heads/to-be-deleted > > + prepared > > + $POST_OID $ZERO_OID refs/heads/to-be-deleted > > + committed > > + $ZERO_OID $ZERO_OID refs/heads/to-be-deleted > > + committed > > + $POST_OID $ZERO_OID refs/heads/to-be-deleted > > + EOF > > + > > + test_cmp expect actual > > +' > > + > > test_done
Attachment:
signature.asc
Description: PGP signature
