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

[Michael Haggerty <mhagger@alum.mit.edu>]

On 02/12/2015 08:36 PM, Junio C Hamano wrote:
> 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?

Good point. I will remove the "-q" from the two grep invocations in this
file.

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

Yes, you're right. I will change the second test to use different tokens.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu