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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 12, 2015, 19:36 UTC
Message-ID: <xmqqoaoysz0o.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1423739543-1025-7-git-send-email-mhagger@alum.mit.edu>

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

Show 24 quoted lines

> 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)"
> +'

Show 7 quoted lines

> +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.

Show 5 quoted lines

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