Re: [PATCH v2 1/2] modernize t7300

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Dec 7, 2015, 22:43 UTC
Message-ID: <CAPig+cRqG3mG89Htmjd3a=YzMG36V3an1qUC4siJNBqHzDHQFA@mail.gmail.com>
In-Reply-To: <1449413906-23256-1-git-send-email-rouzier@gmail.com>

On Sun, Dec 6, 2015 at 9:58 AM, James <rouzier@gmail.com> wrote:

> From: James Rouzier <rouzier@gmail.com>

This would be a good place to explain how you are modernizing the test script. Right now, you're just updating to take advantage of test_path_is_foo(), but some other modernizations you could do include:

* using '>' rather than 'touch' to create empty files when the
timestamp doesn't matter (which is all cases in this script)

* (optional) replace unnecessarily complex 'mkdir -p foo' with simpler
'mkdir foo' (but leave "mkdir -p foo/bar" as is)

* drop blank lines before and after test body; for instance:

    test_expect_success 'foo' '

        blah &&
        bloo

becomes:

    test_expect_success 'foo' '
        blah &&
        bloo
    '

Show 18 quoted lines

> ---
> diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
> @@ -609,32 +609,30 @@ test_expect_success 'force removal of nested git work tree' '
>  test_expect_success 'git clean -e' '
>         rm -fr repo &&
>         mkdir repo &&
> -       (
> -               cd repo &&
> -               git init &&
> -               touch known 1 2 3 &&
> -               git add known &&
> -               git clean -f -e 1 -e 2 &&
> -               test -e 1 &&
> -               test -e 2 &&
> -               ! (test -e 3) &&
> -               test -e known
> -       )
> +       cd repo &&

The previous working directory is not automatically restored at the end of the test, so this "cd" without corresponding "cd .." causes following tests to execute at the incorrect location. Unfortunately, you can't just place "cd .." at the end of a test since it will be skipped if something before the "cd .." fails. The way the existing code deals with this is by using a subshell:

    mkdir repo &&
    (
        cd repo &&
        ...
    )

The "cd repo" is inside the subshell, so it doesn't affect the parent shell, and when the subshell exits, the parent shell remains at the correct working directory (regardless of whether the test succeeded or failed). Therefore, you don't want to drop the subshell.

Show 9 quoted lines

> +       git init &&
> +       touch known 1 2 3 &&
> +       git add known &&
> +       git clean -f -e 1 -e 2 &&
> +       test_path_is_file 1 &&
> +       test_path_is_file 2 &&
> +       test_path_is_missing 3 &&
> +       test_path_is_file known
>  '