Re: [PATCH v4 8/8] [Newcomer] t7004: Use single quotes instead of double quotes

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 6, 2024, 03:02 UTC
Message-ID: <CAPig+cSzq+6bua=T7w0M-+7+2zbzfD6ihXLkWfondZ8NbqUG4A@mail.gmail.com>
In-Reply-To: <20240805235917.190699-9-abdobngad@gmail.com>

On Mon, Aug 5, 2024 at 8:00 PM AbdAlRahman Gad <abdobngad@gmail.com> wrote:

Show 15 quoted lines

> Some test bodies and test description are surrounded with double
> quotes instead of single quotes, violating our coding style.
>
> Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>
> ---
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> @@ -1585,7 +1585,7 @@ test_expect_success 'creating third commit without tag' '
> -test_expect_success 'checking that first commit is in all tags (hash)' "
> +test_expect_success 'checking that first commit is in all tags (hash)' '
>         hash3=$(git rev-parse HEAD) &&
>         ...
>         git tag -l --contains $hash1 v* >actual &&
>         test_cmp expected actual
> -"
> +'

We need to exercise a bit of caution when making this sort of change because a "..." string differs from a '...' string in shell. For instance, in a "..." string, interpolation occurs as the string is scanned by the shell, whereas a '...' string is taken as a literal. Thus, in the above, when it was using a double-quoted string, expressions such as `$(git rev-parse HEAD)` and `$hash1` were being evaluated and interpolated into the string _before_ the test_expect_success() function is even called. On the other hand, with a single-quoted string, those expression do not get evaluated when the string is scanned, thus remain as-is when passed to test_expect_success() as an argument, and they are evaluated only later when test_expect_success() invokes `eval` on that argument. Depending upon the situation, this difference in evaluation context may have a significant impact.

As a practical example, consider a test with a body like this:

    echo nothing >nothing &&
    git add nothing &&
    git commit -m nothing &&
    hash=$(git rev-parse HEAD) &&
    ...

If this body is inside a double-quoted string, then `$(git rev-parse HEAD)` will be evaluated and its value assigned to `hash` _before_ test_expect_success() is called, thus also before the `git commit` command inside the test body (which is almost certainly not what the author intended). On the other hand, inside a single-quoted string, `$(git rev-parse HEAD)` will be evaluated only once test_expect_success() runs the test body, meaning `$(git rev-parse HEAD)` will correctly be evaluated _after_ `git commit`. Hence, `hash` will have a different value depending upon whether single- or double-quotes are used.

Having said all that, in this case, you seem to have lucked out and nothing broke by changing the double-quotes to single-quotes around the test bodies.

However...

Show 18 quoted lines

> @@ -1800,16 +1800,16 @@ test_expect_success 'mixing incompatibles modes and options is forbidden' '
>  for option in --contains --with --no-contains --without --merged --no-merged --points-at
>  do
> -       test_expect_success "mixing incompatible modes with $option is forbidden" "
> +       test_expect_success 'mixing incompatible modes with $option is forbidden' '
>                 test_must_fail git tag -d $option HEAD &&
>                 test_must_fail git tag -d $option HEAD some-tag &&
>                 test_must_fail git tag -v $option HEAD
> -       "
> -       test_expect_success "Doing 'git tag --list-like $option <commit> <pattern> is permitted" "
> +       '
> +       test_expect_success 'Doing "git tag --list-like $option <commit> <pattern> is permitted' '
>                 git tag -n $option HEAD HEAD &&
>                 git tag $option HEAD HEAD &&
>                 git tag $option
> -       "
> +       '
>  done

... changing the double-quotes to single-quotes for the test _titles_ in these instances is actively wrong. In this case, we _want_ interpolation of `$option` to happen in the title string so that the output looks like this:

    ok 167 - mixing incompatible modes with --contains is forbidden
    ok 169 - mixing incompatible modes with --with is forbidden
    ok 171 - mixing incompatible modes with --no-contains is forbidden

By changing the title to use single-quotes, you suppress interpolation of `$option`, with the result that the displayed titles become rather useless:

    ok 167 - mixing incompatible modes with $option is forbidden
    ok 169 - mixing incompatible modes with $option is forbidden
    ok 171 - mixing incompatible modes with $option is forbidden