Re: [PATCH v2 06/12] commit: add tests of commit races

[Junio C Hamano <gitster@pobox.com>]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> diff --git a/t/t7516-commit-races.sh b/t/t7516-commit-races.sh
> new file mode 100755
> index 0000000..08e6a6c
> --- /dev/null
> +++ b/t/t7516-commit-races.sh
> @@ -0,0 +1,33 @@
> +#!/bin/sh
> +
> +test_description='git commit races'
> +. ./test-lib.sh
> +
> +test_tick
> +
> +test_expect_success 'set up editor' '
> +	write_script editor <<-\EOF
> +	git commit --allow-empty -m hare
> +	echo tortoise >"$1"
> +	EOF
> +'
> +
> +test_expect_failure 'race to create orphan commit' '
> +	test_must_fail env EDITOR=./editor git commit --allow-empty &&
> +	git show -s --pretty=format:%s >subject &&
> +	grep -q hare subject &&

Why "grep -q" in the test?  Normal invocation of the tests will hide
the output anyway, no?

Wouldn't letting "sh tDDDD-name-of-test.sh -v" show the output
better for those who are hunting for breakages to see at which step
of the &&-chain things break?

> +	test -z "$(git show -s --pretty=format:%P)"
> +'

> +test_expect_success 'race to create non-orphan commit' '
> +	git checkout --orphan branch &&
> +	git commit --allow-empty -m base &&
> +	git rev-parse HEAD >base &&
> +	test_must_fail env EDITOR=./editor git commit --allow-empty &&
> +	git show -s --pretty=format:%s >subject &&
> +	grep -q hare subject &&

Can we use a token different from hare and tortoise here?  If the
previous one worked correctly, the main "commit" process would have
failed to add 'tortoise' on top of 'hare' that raced from sideways
(which is simulated by making 'hare' from the editor), so the tip of
the history would be 'hare' when this test starts.  Expecting 'hare'
here makes it unclear if you are expecting _both_ of the competing
processes to fail (i.e. the main 'commit' fails to add 'tortoise'
and the racing 'commit' fails to do 'hare'), leaving the 'hare' the
previous test left at the tip of the history, or if you are expecting
that the competing one that tries to create the second 'hare' on top
of the existing 'hare' to win.

> +	git rev-parse HEAD^ >parent &&
> +	test_cmp base parent
> +'
> +
> +test_done