Re: [PATCH v3 3/6] update-ref: add support for 'symref-delete' command
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 5, 2024, 16:22 UTC
- Message-ID
- <xmqqv82nuzpv.fsf@gitster.g>
- In-Reply-To
- <ZmAbgGfNpZzU76pd@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
Show 33 quoted lines
>> + test_expect_success "stdin $type symref-delete fails with wrong old value" '
>> + format_command $type "symref-delete refs/heads/symref" "$m" >stdin &&
>> + test_must_fail git update-ref --stdin $type --no-deref <stdin 2>err &&
>> + grep "fatal: verifying symref target: ${SQ}refs/heads/symref${SQ}: is at $a but expected refs/heads/main" err &&
>> + git symbolic-ref refs/heads/symref >expect &&
>> + echo $a >actual &&
>> + test_cmp expect actual
>> + '
>> +
>> + test_expect_success "stdin $type symref-delete works with right old value" '
>> + format_command $type "symref-delete refs/heads/symref" "$a" >stdin &&
>> + git update-ref --stdin $type --no-deref <stdin &&
>> + test_must_fail git rev-parse --verify -q refs/heads/symref
>> + '
>> +
>> + test_expect_success "stdin $type symref-delete works with empty old value" '
>> + git symbolic-ref refs/heads/symref $a >stdin &&
>> + format_command $type "symref-delete refs/heads/symref" "" >stdin &&
>> + git update-ref --stdin $type --no-deref <stdin &&
>> + test_must_fail git rev-parse --verify -q $b
>> + '
>> +
>> + test_expect_success "stdin $type symref-delete succeeds for dangling reference" '
>> + test_must_fail git symbolic-ref refs/heads/nonexistent &&
>> + git symbolic-ref refs/heads/symref2 refs/heads/nonexistent &&
>> + format_command $type "symref-delete refs/heads/symref2" "refs/heads/nonexistent" >stdin &&
>> + git update-ref --stdin $type --no-deref <stdin &&
>> + test_must_fail git symbolic-ref -d refs/heads/symref2
>> + '
>> +
>
> Not sure whether I overlooked it, but one test I missed was when trying
> to delete a non-symbolic-ref.Hmph, so we want to see an attempt to run symref-delete of refs/heads/main when refs/heads/main is *not* a symref to fail? That is a reasonable test to have.
When you invent a shiny new toy, it is hard to make sure you covered cases where it should not kick in or positively diagnose a failure. A review with a sharp eye to spot missing tests is very much appreciated.
THanks.