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

From: Stefan Beller <sbeller@google.com>
Date: Feb 12, 2015, 18:13 UTC
Message-ID: <CAGZ79kbz5+CNO3fgy+UyY1z45a+cTkhtroRbHSNdmUWdQdKeqQ@mail.gmail.com>
In-Reply-To: <1423739543-1025-7-git-send-email-mhagger@alum.mit.edu>

On Thu, Feb 12, 2015 at 3:12 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:

Show 38 quoted lines

> Committing involves the following steps:
>
> 1. Determine the current value of HEAD (if any).
> 2. Create the new commit object.
> 3. Update HEAD.
>
> Please note that step 2 can take arbitrarily long, because it might
> involve the user editing a commit message.
>
> If a second process sneaks in a commit during step 2, then the first
> commit process should fail. This is usually done correctly, because
> step 3 verifies that HEAD still points at the same commit that it
> pointed to during step 1.
>
> However, if there is a race when creating an *orphan* commit, then the
> test in step 3 is skipped.
>
> Add tests for proper handling of such races. One of the new tests
> fails. It will be fixed in a moment.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
>  t/t7516-commit-races.sh | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100755 t/t7516-commit-races.sh
>
> 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

So I am wondering why we need to have a test_tick here. In case we need to pass simulation time after loading the test-lib, we should rather have it inside the test-lib.

If we need to pass time specifically for this test (I just don't understand why at this point of execution), then we should move it more to the relevant place inside the &&-chain as all other tests have test_tick inside a test chained up with &&.

Show 30 quoted lines

> +
> +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 &&
> +       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 &&
> +       git rev-parse HEAD^ >parent &&
> +       test_cmp base parent
> +'
> +
> +test_done
> --
> 2.1.4
>