Re: [PATCH] refs: handle null-oid for pseudorefs

From: Martin Ågren <martin.agren@gmail.com>
Date: May 7, 2018, 10:05 UTC
Message-ID: <CAN0heSp-rxqAVJ3Q1KMD=eYqPUkcDP8xBTVTDtGfom6v5WpBLQ@mail.gmail.com>
In-Reply-To: <1291dbc0-31f9-213a-ae30-1a6349ea1626@alum.mit.edu>

On 7 May 2018 at 09:39, Michael Haggerty <mhagger@alum.mit.edu> wrote:

Show 15 quoted lines

> Thanks for the patch. This looks good to me. But it it seems that the
> test coverage related to pseudorefs is still not great. Ideally, all of
> the following combinations should be tested:
>
> Pre-update value   | ref-update old OID   | Expected result
> -------------------|----------------------|----------------
> missing            | missing              | accept *
> missing            | value                | reject
> set                | missing              | reject *
> set                | correct value        | accept
> set                | wrong value          | reject
>
> I think your test only covers the lines with asterisks. Are the other
> scenarios already covered by other tests? If not, how about adding them?
> That would give us confidence that the new code works in all circumstances.

Thank you for your comments. I was not able to find much pseudoref-testing. I think what I should do is a patch 1/2 adding the tests you outlined (some will be expected failures), then turn this patch into a patch 2/2.

Martin