Re: [PATCH] t/: work around one-shot variable assignment with test_must_fail
Hi,
2010/7/20 Brandon Casey <casey@nrlssc.navy.mil>
Show 7 quoted lines
>
> 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.
Show 38 quoted lines
> $ ./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.
Show 119 quoted lines
>
> --->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
Erick Mattos <erick.mattos@gmail.com>