On Mon, Aug 5, 2024 at 8:00 PM AbdAlRahman Gad <abdobngad@xxxxxxxxx> wrote:
> 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@xxxxxxxxx>
> ---
> 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...
> @@ -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
