git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Feb 17, 2015, 15:06 UTC
Message-ID
<54E358FD.1090304@alum.mit.edu>
In-Reply-To
<xmqqoaoysz0o.fsf@gitster.dls.corp.google.com>
On 02/12/2015 08:36 PM, Junio C Hamano wrote:
Show 33 quoted lines
> 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.

Show 22 quoted lines
>> +	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
Previous: Junio C HamanoNext: Michael Haggerty
Message 18 of 27 in “Allow reference values to be checked in a transaction”
  1. 00/12 Allow reference values to be checked in a transactionMichael Haggerty, Feb 12, 2015
  2. 01/12 refs: move REF_DELETING to refs.cMichael Haggerty, Feb 12, 2015
  3. 02/12 refs: remove the gap in the REF_* constant valuesMichael Haggerty, Feb 12, 2015
  4. 03/12 struct ref_update: move "have_old" into "flags"Michael Haggerty, Feb 12, 2015
  5. Stefan BellerFeb 12, 2015
  6. Junio C HamanoFeb 12, 2015
  7. Michael HaggertyFeb 17, 2015
  8. Junio C HamanoFeb 17, 2015
  9. 04/12 ref_transaction_update(): remove "have_old" parameterMichael Haggerty, Feb 12, 2015
  10. Junio C HamanoFeb 12, 2015
  11. Michael HaggertyFeb 13, 2015
  12. Junio C HamanoFeb 13, 2015
  13. 05/12 ref_transaction_delete(): remove "have_old" parameterMichael Haggerty, Feb 12, 2015
  14. 06/12 commit: add tests of commit racesMichael Haggerty, Feb 12, 2015
  15. Stefan BellerFeb 12, 2015
  16. Michael HaggertyFeb 17, 2015
  17. Junio C HamanoFeb 12, 2015
  18. Michael HaggertyFeb 17, 2015
  19. 07/12 commit: avoid race when creating orphan commitsMichael Haggerty, Feb 12, 2015
  20. Junio C HamanoFeb 12, 2015
  21. 08/12 ref_transaction_create(): check that new_sha1 is validMichael Haggerty, Feb 12, 2015
  22. 09/12 ref_transaction_delete(): check that old_sha1 is not null_sha1Michael Haggerty, Feb 12, 2015
  23. 10/12 ref_transaction_verify(): new function to check a reference's valueMichael Haggerty, Feb 12, 2015
  24. Stefan BellerFeb 12, 2015
  25. 11/12 update_ref(): improve documentationMichael Haggerty, Feb 12, 2015
  26. 12/12 refs.h: Remove duplication in function docstringsMichael Haggerty, Feb 12, 2015
  27. Junio C HamanoFeb 12, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.