diff mbox series

[4/6] t5616: use rev-parse instead to get HEAD's object_id

Message ID b9beadcc510301ba385f1b01b653024c8f369b49.1584625896.git.congdanhqx@gmail.com (mailing list archive)
State New, archived
Headers show
Series fix test failure with busybox | expand

Commit Message

Đoàn Trần Công Danh March 19, 2020, 2 p.m. UTC
Only HEAD's object_id is necessary, rev-list is an overkill.

Despite POSIX requires grep(1) treat single pattern with <newline>
as multiple patterns.
busybox's grep(1) (as of v1.31.1) haven't implemented it yet.

Use rev-parse to simplify the test and avoid busybox unimplemented
features.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 t/t5616-partial-clone.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jeff King March 19, 2020, 4:07 p.m. UTC | #1
On Thu, Mar 19, 2020 at 09:00:05PM +0700, Đoàn Trần Công Danh wrote:

> Only HEAD's object_id is necessary, rev-list is an overkill.
> 
> Despite POSIX requires grep(1) treat single pattern with <newline>
> as multiple patterns.
> busybox's grep(1) (as of v1.31.1) haven't implemented it yet.
> 
> Use rev-parse to simplify the test and avoid busybox unimplemented
> features.

That makes sense. It would also future-proof us against the test
changing the graph such that HEAD actually has ancestors.

> diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
> index 77bb91e976..135187c5b5 100755
> --- a/t/t5616-partial-clone.sh
> +++ b/t/t5616-partial-clone.sh
> @@ -49,7 +49,7 @@ test_expect_success 'do partial clone 1' '
>  test_expect_success 'verify that .promisor file contains refs fetched' '
>  	ls pc1/.git/objects/pack/pack-*.promisor >promisorlist &&
>  	test_line_count = 1 promisorlist &&
> -	git -C srv.bare rev-list HEAD >headhash &&
> +	git -C srv.bare rev-parse HEAD >headhash &&

Maybe worth using "rev-parse --verify" which would double check that we
produced a useful hash (it seems like an unlikely failure mode, but it's
easy enough to cover).

-Peff
Đoàn Trần Công Danh March 20, 2020, 12:57 a.m. UTC | #2
On 2020-03-19 12:07:07-0400, Jeff King <peff@peff.net> wrote:
> On Thu, Mar 19, 2020 at 09:00:05PM +0700, Đoàn Trần Công Danh wrote:
> 
> > Only HEAD's object_id is necessary, rev-list is an overkill.
> > 
> > Despite POSIX requires grep(1) treat single pattern with <newline>
> > as multiple patterns.
> > busybox's grep(1) (as of v1.31.1) haven't implemented it yet.
> > 
> > Use rev-parse to simplify the test and avoid busybox unimplemented
> > features.
> 
> That makes sense. It would also future-proof us against the test
> changing the graph such that HEAD actually has ancestors.
> 
> > diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
> > index 77bb91e976..135187c5b5 100755
> > --- a/t/t5616-partial-clone.sh
> > +++ b/t/t5616-partial-clone.sh
> > @@ -49,7 +49,7 @@ test_expect_success 'do partial clone 1' '
> >  test_expect_success 'verify that .promisor file contains refs fetched' '
> >  	ls pc1/.git/objects/pack/pack-*.promisor >promisorlist &&
> >  	test_line_count = 1 promisorlist &&
> > -	git -C srv.bare rev-list HEAD >headhash &&
> > +	git -C srv.bare rev-parse HEAD >headhash &&
> 
> Maybe worth using "rev-parse --verify" which would double check that we
> produced a useful hash (it seems like an unlikely failure mode, but it's
> easy enough to cover).

Yeah, I also thought it wasn't expected to be a failure in this case.
Using `--verify` doesn't cost anything in this case, though.
I will add it in the reroll.
diff mbox series

Patch

diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index 77bb91e976..135187c5b5 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -49,7 +49,7 @@  test_expect_success 'do partial clone 1' '
 test_expect_success 'verify that .promisor file contains refs fetched' '
 	ls pc1/.git/objects/pack/pack-*.promisor >promisorlist &&
 	test_line_count = 1 promisorlist &&
-	git -C srv.bare rev-list HEAD >headhash &&
+	git -C srv.bare rev-parse HEAD >headhash &&
 	grep "$(cat headhash) HEAD" $(cat promisorlist) &&
 	grep "$(cat headhash) refs/heads/master" $(cat promisorlist)
 '