Re: [PATCH 02/11] t: allow skipping expected object ID in `ref-store update-ref`

From: Patrick Steinhardt <ps@pks.im>
Date: Oct 23, 2023, 13:58 UTC
Message-ID: <ZTZ7-oMqek8kQqhJ@tanuki>
In-Reply-To: <xmqqil73ud5j.fsf@gitster.g>

On Wed, Oct 18, 2023 at 09:08:08AM -0700, Junio C Hamano wrote:

Show 19 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> 
> > We require the caller to pass both the old and new expected object ID to
> > our `test-tool ref-store update-ref` helper. When trying to update a
> > symbolic reference though it's impossible to specify the expected object
> > ID, which means that the test would instead have to force-update the
> > reference. This is currently impossible though.
> >
> > Update the helper to optionally skip verification of the old object ID
> > in case the test passes in an empty old object ID as input.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  t/helper/test-ref-store.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> Good.
> 
> Even better would be to make the old one optional, though.

I was also a bit torn when writing this. We could of course make the behaviour conditional on whether `argc` is 4 or 5. But I wasn't quite sure how important it is to provide a nice UI for this test helper, and we don't have `argc` readily available. It's not hard to count them manually, but until now I was under the impression that the test helpers only need to be "good enough".

Patrick

Show 28 quoted lines

> > diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
> > index 7400f560ab6..7dc83137584 100644
> > --- a/t/helper/test-ref-store.c
> > +++ b/t/helper/test-ref-store.c
> > @@ -322,16 +322,19 @@ static int cmd_update_ref(struct ref_store *refs, const char **argv)
> >  	const char *new_sha1_buf = notnull(*argv++, "new-sha1");
> >  	const char *old_sha1_buf = notnull(*argv++, "old-sha1");
> >  	unsigned int flags = arg_flags(*argv++, "flags", transaction_flags);
> > -	struct object_id old_oid;
> > +	struct object_id old_oid, *old_oid_ptr = NULL;
> >  	struct object_id new_oid;
> >  
> > -	if (get_oid_hex(old_sha1_buf, &old_oid))
> > -		die("cannot parse %s as %s", old_sha1_buf, the_hash_algo->name);
> > +	if (*old_sha1_buf) {
> > +		if (get_oid_hex(old_sha1_buf, &old_oid))
> > +			die("cannot parse %s as %s", old_sha1_buf, the_hash_algo->name);
> > +		old_oid_ptr = &old_oid;
> > +	}
> >  	if (get_oid_hex(new_sha1_buf, &new_oid))
> >  		die("cannot parse %s as %s", new_sha1_buf, the_hash_algo->name);
> >  
> >  	return refs_update_ref(refs, msg, refname,
> > -			       &new_oid, &old_oid,
> > +			       &new_oid, old_oid_ptr,
> >  			       flags, UPDATE_REFS_DIE_ON_ERR);
> >  }
>