From: Erick Mattos <erick.mattos@gmail.com>
Date: Tue, 20 Jul 2010 23:19:02 GMT
Subject: Re: [PATCH] t/: work around one-shot variable assignment with test_must_fail
Message-ID: <AANLkTilY0fYxKlWLYGU4f4ZtJzsMSSnIIi0YnywKU5DU@mail.gmail.com>
In-Reply-To: <iU5XdZGtMeaspoCqSJIp6Y--60TPVkZUrm3SdW86dsTZkNYZWqbSppLBrMXyL1rVqqYtHm94ACo@cipher.nrlssc.navy.mil>

Hi,

2010/7/20 Brandon Casey <casey@nrlssc.navy.mil>
>
> No time to investigate, but here is an example patch and the
> results of running the affected tests.  Looks like reflog may
> be creating a reflog when it is not supposed to.
>
> Erick, I added you to cc since you appear to be the author of the tests
> in question.

Thanks for your kindness.

> $ ./t2017-checkout-orphan.sh
> <snip>
> not ok - 8 --orphan does not make reflog when core.logAllRefUpdates = false
> #
> #               git checkout master &&
> #               git config core.logAllRefUpdates false &&
> #               git checkout --orphan epsilon &&
> #               ! test -f .git/logs/refs/heads/epsilon &&
> #               (
> #                       PAGER= &&
> #                       export PAGER &&
> #                       test_must_fail git reflog show epsilon
> #               ) &&
> #               git commit -m Epsilon &&
> #               ! test -f .git/logs/refs/heads/epsilon &&
> #               (
> #                       PAGER= &&
> #                       export PAGER &&
> #                       test_must_fail git reflog show epsilon
> #               )
> #
> <snip>
>
> $ ./t3200-branch.sh
> <snip>
> not ok - 32 checkout -b does not make reflog when core.logAllRefUpdates = false
> #
> #               git checkout master &&
> #               git config core.logAllRefUpdates false &&
> #               git checkout -b beta &&
> #               ! test -f .git/logs/refs/heads/beta &&
> #               (
> #                       PAGER= &&
> #                       export PAGER &&
> #                       test_must_fail git reflog show beta
> #               )
> #
> <snip>

You have made cosmetic changes which do not do the same as the original.

>
> --->8---
> From: Brandon Casey <drafnel@gmail.com>
>
> See e2007832552ccea9befed9003580c494f09e666e
> ---
>  t/t2017-checkout-orphan.sh |   36 ++++++++++++++++++++++++++++++------
>  t/t3200-branch.sh          |    6 +++++-
>  t/t3301-notes.sh           |    6 +++++-
>  3 files changed, 40 insertions(+), 8 deletions(-)
>
> diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh
> index be88d4b..81cb393 100755
> --- a/t/t2017-checkout-orphan.sh
> +++ b/t/t2017-checkout-orphan.sh
> @@ -69,7 +69,11 @@ test_expect_success '--orphan makes reflog by default' '
>        git config --unset core.logAllRefUpdates &&
>        git checkout --orphan delta &&
>        ! test -f .git/logs/refs/heads/delta &&
> -       test_must_fail PAGER= git reflog show delta &&
> +       (
> +               PAGER= &&
> +               export PAGER &&
> +               test_must_fail git reflog show delta
> +       ) &&
>        git commit -m Delta &&
>        test -f .git/logs/refs/heads/delta &&
>        PAGER= git reflog show delta
> @@ -80,17 +84,29 @@ test_expect_success '--orphan does not make reflog when core.logAllRefUpdates =
>        git config core.logAllRefUpdates false &&
>        git checkout --orphan epsilon &&
>        ! test -f .git/logs/refs/heads/epsilon &&
> -       test_must_fail PAGER= git reflog show epsilon &&
> +       (
> +               PAGER= &&
> +               export PAGER &&
> +               test_must_fail git reflog show epsilon
> +       ) &&
>        git commit -m Epsilon &&
>        ! test -f .git/logs/refs/heads/epsilon &&
> -       test_must_fail PAGER= git reflog show epsilon
> +       (
> +               PAGER= &&
> +               export PAGER &&
> +               test_must_fail git reflog show epsilon
> +       )
>  '
>
>  test_expect_success '--orphan with -l makes reflog when core.logAllRefUpdates = false' '
>        git checkout master &&
>        git checkout -l --orphan zeta &&
>        test -f .git/logs/refs/heads/zeta &&
> -       test_must_fail PAGER= git reflog show zeta &&
> +       (
> +               PAGER= &&
> +               export PAGER &&
> +               test_must_fail git reflog show zeta
> +       ) &&
>        git commit -m Zeta &&
>        PAGER= git reflog show zeta
>  '
> @@ -99,10 +115,18 @@ test_expect_success 'giving up --orphan not committed when -l and core.logAllRef
>        git checkout master &&
>        git checkout -l --orphan eta &&
>        test -f .git/logs/refs/heads/eta &&
> -       test_must_fail PAGER= git reflog show eta &&
> +       (
> +               PAGER= &&
> +               export PAGER &&
> +               test_must_fail git reflog show eta
> +       ) &&
>        git checkout master &&
>        ! test -f .git/logs/refs/heads/eta &&
> -       test_must_fail PAGER= git reflog show eta
> +       (
> +               PAGER= &&
> +               export PAGER &&
> +               test_must_fail git reflog show eta
> +       )
>  '
>
>  test_expect_success '--orphan is rejected with an existing name' '
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 859b99a..bf7747d 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -237,7 +237,11 @@ test_expect_success 'checkout -b does not make reflog when core.logAllRefUpdates
>        git config core.logAllRefUpdates false &&
>        git checkout -b beta &&
>        ! test -f .git/logs/refs/heads/beta &&
> -       test_must_fail PAGER= git reflog show beta
> +       (
> +               PAGER= &&
> +               export PAGER &&
> +               test_must_fail git reflog show beta
> +       )
>  '
>
>  test_expect_success 'checkout -b with -l makes reflog when core.logAllRefUpdates = false' '
> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
> index 2d67a40..1d82f79 100755
> --- a/t/t3301-notes.sh
> +++ b/t/t3301-notes.sh
> @@ -693,7 +693,11 @@ test_expect_success 'create note from non-existing note with "git notes add -c"
>        git add a10 &&
>        test_tick &&
>        git commit -m 10th &&
> -       test_must_fail MSG="yet another note" git notes add -c deadbeef &&
> +       (
> +               MSG="yet another note" &&
> +               export MSG &&
> +               test_must_fail git notes add -c deadbeef
> +       ) &&
>        test_must_fail git notes list HEAD
>  '
>
> --
> 1.6.6.2
>

I don't like this proposed patch because I don't see an improvement
when:
* you change a line to three;
* the original line is clear enough;
* the resulting lines improves nothing;
* it is for cosmetic purposes only inside a script;
* and it uses more resources, even if little more as by creating a
  subshell environment, to do the same.

Best regards