{"thread":{"id":"36186","subject":"[PATCH] Removed subshell invocations in many of the tests when possible","startedAt":"2014-03-16T07:40:01Z","lastAt":"2014-03-17T10:19:16Z","messageCount":2,"participants":["David Tran","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"236837","messageId":"1394955601-18829-1-git-send-email-unsignedzero@gmail.com","threadId":"36186","inReplyTo":null,"subject":"[PATCH] Removed subshell invocations in many of the tests when possible","fromName":"David Tran","fromEmail":"unsignedzero@gmail.com","sentAt":"2014-03-16T07:40:01Z","receivedAt":"2014-03-16T07:40:01Z","isPatch":true,"sender":{"key":"unsignedzero@gmail.com","avatar":"https://avatars.githubusercontent.com/u/778125?v=4"},"body":"I am David Tran a graduating CS/Math senior from Sonoma State University,\nUnited States. I would like to work with git for GSoC'14, specifically the line\noptions for git rebase --interactive. I have used git for a few years and know\nhow destructive but important rebase is to git. I have created a few shell\nscripts here and there to make life using bash/zsh easier. I would like to\napply these skills and work with the best.\n\nI've submitted my application yesterday and my patch didn't send correctly.\n\nSigned-off-by: David Tran <unsignedzero@gmail.com>\n---\n t/t1300-repo-config.sh        |   17 +++---------\n t/t1510-repo-setup.sh         |    4 +--\n t/t3200-branch.sh             |   12 +-------\n t/t3301-notes.sh              |   24 ++++++------------\n t/t3404-rebase-interactive.sh |   55 +++++++++--------------------------------\n t/t3413-rebase-hook.sh        |    6 +---\n t/t4014-format-patch.sh       |   14 ++--------\n t/t5305-include-tag.sh        |    4 +--\n t/t5602-clone-remote-exec.sh  |   13 ++-------\n t/t5801-remote-helpers.sh     |    6 +---\n t/t6006-rev-list-format.sh    |    3 +-\n t/t7006-pager.sh              |   18 ++-----------\n 12 files changed, 41 insertions(+), 135 deletions(-)\n\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex c9c426c..3e3f77b 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' '\n '\n \n test_expect_success 'nonexistent configuration' '\n-\t(\n-\t\tGIT_CONFIG=doesnotexist &&\n-\t\texport GIT_CONFIG &&\n-\t\ttest_must_fail git config --list &&\n-\t\ttest_must_fail git config test.xyzzy\n-\t)\n+\ttest_must_fail env GIT_CONFIG=doesnotexist git config --list &&\n+\ttest_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy\n '\n \n test_expect_success SYMLINKS 'symlink to nonexistent configuration' '\n \tln -s doesnotexist linktonada &&\n \tln -s linktonada linktolinktonada &&\n-\t(\n-\t\tGIT_CONFIG=linktonada &&\n-\t\texport GIT_CONFIG &&\n-\t\ttest_must_fail git config --list &&\n-\t\tGIT_CONFIG=linktolinktonada &&\n-\t\ttest_must_fail git config --list\n-\t)\n+\ttest_must_fail env GIT_CONFIG=linktonada git config --list &&\n+\ttest_must_fail env GIT_CONFIG=linktolinktonada git config --list\n '\n \n test_expect_success 'check split_cmdline return' \"\ndiff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh\nindex cf2ee78..d8025be 100755\n--- a/t/t1510-repo-setup.sh\n+++ b/t/t1510-repo-setup.sh\n@@ -777,9 +777,7 @@ test_expect_success '#30: core.worktree and core.bare conflict (gitfile version)\n \tsetup_repo 30 \"$here/30\" gitfile true &&\n \t(\n \t\tcd 30 &&\n-\t\tGIT_DIR=.git &&\n-\t\texport GIT_DIR &&\n-\t\ttest_must_fail git symbolic-ref HEAD 2>result\n+\t\ttest_must_fail env GIT_DIR='.git' git symbolic-ref HEAD 2>result\n \t) &&\n \tgrep \"core.bare and core.worktree\" 30/result\n '\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex fcdb867..d45e95c 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -849,11 +849,7 @@ test_expect_success 'detect typo in branch name when using --edit-description' '\n \twrite_script editor <<-\\EOF &&\n \t\techo \"New contents\" >\"$1\"\n \tEOF\n-\t(\n-\t\tEDITOR=./editor &&\n-\t\texport EDITOR &&\n-\t\ttest_must_fail git branch --edit-description no-such-branch\n-\t)\n+\ttest_must_fail env EDITOR=./editor git branch --edit-description no-such-branch\n '\n \n test_expect_success 'refuse --edit-description on unborn branch for now' '\n@@ -861,11 +857,7 @@ test_expect_success 'refuse --edit-description on unborn branch for now' '\n \t\techo \"New contents\" >\"$1\"\n \tEOF\n \tgit checkout --orphan unborn &&\n-\t(\n-\t\tEDITOR=./editor &&\n-\t\texport EDITOR &&\n-\t\ttest_must_fail git branch --edit-description\n-\t)\n+\ttest_must_fail env EDITOR=./editor git branch --edit-description\n '\n \n test_expect_success '--merged catches invalid object names' '\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 3bb79a4..ca1fea9 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -17,7 +17,7 @@ GIT_EDITOR=./fake_editor.sh\n export GIT_EDITOR\n \n test_expect_success 'cannot annotate non-existing HEAD' '\n-\t(MSG=3 && export MSG && test_must_fail git notes add)\n+\ttest_must_fail env MSG=3 git notes add\n '\n \n test_expect_success setup '\n@@ -32,22 +32,18 @@ test_expect_success setup '\n '\n \n test_expect_success 'need valid notes ref' '\n-\t(MSG=1 GIT_NOTES_REF=/ && export MSG GIT_NOTES_REF &&\n-\t test_must_fail git notes add) &&\n-\t(MSG=2 GIT_NOTES_REF=/ && export MSG GIT_NOTES_REF &&\n-\t test_must_fail git notes show)\n+\ttest_must_fail env MSG=1 env GIT_NOTES_REF=/ git notes show &&\n+\ttest_must_fail env MSG=2 env GIT_NOTES_REF=/ git notes show\n '\n \n test_expect_success 'refusing to add notes in refs/heads/' '\n-\t(MSG=1 GIT_NOTES_REF=refs/heads/bogus &&\n-\t export MSG GIT_NOTES_REF &&\n-\t test_must_fail git notes add)\n+\ttest_must_fail env MSG=1 env GIT_NOTES_REF=refs/heads/bogus \\\n+\t git notes add\n '\n \n test_expect_success 'refusing to edit notes in refs/remotes/' '\n-\t(MSG=1 GIT_NOTES_REF=refs/remotes/bogus &&\n-\t export MSG GIT_NOTES_REF &&\n-\t test_must_fail git notes edit)\n+\ttest_must_fail env MSG=1 env GIT_NOTES_REF=refs/heads/bogus \\\n+\t git notes edit\n '\n \n # 1 indicates caught gracefully by die, 128 means git-show barked\n@@ -865,11 +861,7 @@ test_expect_success 'create note from non-existing note with \"git notes add -c\"\n \tgit add a10 &&\n \ttest_tick &&\n \tgit commit -m 10th &&\n-\t(\n-\t\tMSG=\"yet another note\" &&\n-\t\texport MSG &&\n-\t\ttest_must_fail git notes add -c deadbeef\n-\t) &&\n+\ttest_must_fail env MSG=\"yet another note\" git notes add -c deadbeef\n \ttest_must_fail git notes list HEAD\n '\n \ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 50e22b1..842a47a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -104,9 +104,7 @@ test_expect_success 'rebase -i with the exec command checks tree cleanness' '\n \tgit checkout master &&\n \t(\n \tset_fake_editor &&\n-\tFAKE_LINES=\"exec_echo_foo_>file1 1\" &&\n-\texport FAKE_LINES &&\n-\ttest_must_fail git rebase -i HEAD^\n+\ttest_must_fail env FAKE_LINES=\"exec_echo_foo_>file1 1\" git rebase -i HEAD^\n \t) &&\n \ttest_cmp_rev master^ HEAD &&\n \tgit reset --hard &&\n@@ -118,9 +116,8 @@ test_expect_success 'rebase -i with exec of inexistent command' '\n \ttest_when_finished \"git rebase --abort\" &&\n \t(\n \tset_fake_editor &&\n-\tFAKE_LINES=\"exec_this-command-does-not-exist 1\" &&\n-\texport FAKE_LINES &&\n-\ttest_must_fail git rebase -i HEAD^ >actual 2>&1\n+\ttest_must_fail env FAKE_LINES=\"exec_this-command-does-not-exist 1\" \\\n+\tgit rebase -i HEAD^ >actual 2>&1\n \t) &&\n \t! grep \"Maybe git-rebase is broken\" actual\n '\n@@ -375,11 +372,7 @@ test_expect_success 'commit message used after conflict' '\n \tgit checkout -b conflict-fixup conflict-branch &&\n \tbase=$(git rev-parse HEAD~4) &&\n \tset_fake_editor &&\n-\t(\n-\t\tFAKE_LINES=\"1 fixup 3 fixup 4\" &&\n-\t\texport FAKE_LINES &&\n-\t\ttest_must_fail git rebase -i $base\n-\t) &&\n+\ttest_must_fail env FAKE_LINES=\"1 fixup 3 fixup 4\" git rebase -i $base &&\n \techo three > conflict &&\n \tgit add conflict &&\n \tFAKE_COMMIT_AMEND=\"ONCE\" EXPECT_HEADER_COUNT=2 \\\n@@ -394,11 +387,7 @@ test_expect_success 'commit message retained after conflict' '\n \tgit checkout -b conflict-squash conflict-branch &&\n \tbase=$(git rev-parse HEAD~4) &&\n \tset_fake_editor &&\n-\t(\n-\t\tFAKE_LINES=\"1 fixup 3 squash 4\" &&\n-\t\texport FAKE_LINES &&\n-\t\ttest_must_fail git rebase -i $base\n-\t) &&\n+\ttest_must_fail env FAKE_LINES=\"1 fixup 3 squash 4\" git rebase -i $base &&\n \techo three > conflict &&\n \tgit add conflict &&\n \tFAKE_COMMIT_AMEND=\"TWICE\" EXPECT_HEADER_COUNT=2 \\\n@@ -469,11 +458,7 @@ test_expect_success 'interrupted squash works as expected' '\n \tgit checkout -b interrupted-squash conflict-branch &&\n \tone=$(git rev-parse HEAD~3) &&\n \tset_fake_editor &&\n-\t(\n-\t\tFAKE_LINES=\"1 squash 3 2\" &&\n-\t\texport FAKE_LINES &&\n-\t\ttest_must_fail git rebase -i HEAD~3\n-\t) &&\n+\ttest_must_fail env FAKE_LINES=\"1 squash 3 2\" git rebase -i HEAD~3 &&\n \t(echo one; echo two; echo four) > conflict &&\n \tgit add conflict &&\n \ttest_must_fail git rebase --continue &&\n@@ -487,11 +472,7 @@ test_expect_success 'interrupted squash works as expected (case 2)' '\n \tgit checkout -b interrupted-squash2 conflict-branch &&\n \tone=$(git rev-parse HEAD~3) &&\n \tset_fake_editor &&\n-\t(\n-\t\tFAKE_LINES=\"3 squash 1 2\" &&\n-\t\texport FAKE_LINES &&\n-\t\ttest_must_fail git rebase -i HEAD~3\n-\t) &&\n+\ttest_must_fail env FAKE_LINES=\"3 squash 1 2\" git rebase -i HEAD~3 &&\n \t(echo one; echo four) > conflict &&\n \tgit add conflict &&\n \ttest_must_fail git rebase --continue &&\n@@ -529,9 +510,7 @@ test_expect_success 'aborted --continue does not squash commits after \"edit\"' '\n \techo \"edited again\" > file7 &&\n \tgit add file7 &&\n \t(\n-\t\tFAKE_COMMIT_MESSAGE=\" \" &&\n-\t\texport FAKE_COMMIT_MESSAGE &&\n-\t\ttest_must_fail git rebase --continue\n+\t\ttest_must_fail env FAKE_COMMIT_MESSAGE=\" \" git rebase --continue\n \t) &&\n \ttest $old = $(git rev-parse HEAD) &&\n \tgit rebase --abort\n@@ -548,9 +527,7 @@ test_expect_success 'auto-amend only edited commits after \"edit\"' '\n \tgit add file7 &&\n \ttest_tick &&\n \t(\n-\t\tFAKE_COMMIT_MESSAGE=\"and again\" &&\n-\t\texport FAKE_COMMIT_MESSAGE &&\n-\t\ttest_must_fail git rebase --continue\n+\t\ttest_must_fail env FAKE_COMMIT_MESSAGE=\"and again\" git rebase --continue\n \t) &&\n \tgit rebase --abort\n '\n@@ -560,9 +537,7 @@ test_expect_success 'clean error after failed \"exec\"' '\n \ttest_when_finished \"git rebase --abort || :\" &&\n \tset_fake_editor &&\n \t(\n-\t\tFAKE_LINES=\"1 exec_false\" &&\n-\t\texport FAKE_LINES &&\n-\t\ttest_must_fail git rebase -i HEAD^\n+\t\ttest_must_fail env FAKE_LINES=\"1 exec_false\" git rebase -i HEAD^\n \t) &&\n \techo \"edited again\" > file7 &&\n \tgit add file7 &&\n@@ -949,9 +924,7 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '\n \tgit checkout B &&\n \t(\n \t\tset_fake_editor &&\n-\t\tFAKE_LINES=\"2\" &&\n-\t\texport FAKE_LINES &&\n-\t\ttest_must_fail git rebase -i --root\n+\t\ttest_must_fail env FAKE_LINES=\"2\" git rebase -i --root\n \t) &&\n \tgit cat-file commit HEAD | grep \"^tree 4b825dc642cb\" &&\n \tgit rebase --abort\n@@ -1042,11 +1015,7 @@ test_expect_success 'rebase -i error on commits with \\ in message' '\n \ttest_when_finished \"git rebase --abort; git reset --hard $current_head; rm -f error\" &&\n \ttest_commit TO-REMOVE will-conflict old-content &&\n \ttest_commit \"\\temp\" will-conflict new-content dummy &&\n-\t(\n-\tEDITOR=true &&\n-\texport EDITOR &&\n-\ttest_must_fail git rebase -i HEAD^ --onto HEAD^^ 2>error\n-\t) &&\n+\ttest_must_fail env EDITOR=true git rebase -i HEAD^ --onto HEAD^^ 2>error\n \ttest_expect_code 1 grep  \"\temp\" error\n '\n \ndiff --git a/t/t3413-rebase-hook.sh b/t/t3413-rebase-hook.sh\nindex 098b755..33560a1 100755\n--- a/t/t3413-rebase-hook.sh\n+++ b/t/t3413-rebase-hook.sh\n@@ -118,11 +118,7 @@ test_expect_success 'pre-rebase hook stops rebase (1)' '\n test_expect_success 'pre-rebase hook stops rebase (2)' '\n \tgit checkout test &&\n \tgit reset --hard side &&\n-\t(\n-\t\tEDITOR=:\n-\t\texport EDITOR\n-\t\ttest_must_fail git rebase -i master\n-\t) &&\n+\ttest_must_fail env EDITOR=: git rebase -i master\n \ttest \"z$(git symbolic-ref HEAD)\" = zrefs/heads/test &&\n \ttest 0 = $(git rev-list HEAD...side | wc -l)\n '\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 73194b2..4a4b943 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -764,22 +764,14 @@ test_expect_success 'format-patch --signature=\"\" suppresses signatures' '\n \n test_expect_success TTY 'format-patch --stdout paginates' '\n \trm -f pager_used &&\n-\t(\n-\t\tGIT_PAGER=\"wc >pager_used\" &&\n-\t\texport GIT_PAGER &&\n-\t\ttest_terminal git format-patch --stdout --all\n-\t) &&\n+\ttest_terminal env GIT_PAGER=\"wc >pager_used\" git format-patch --stdout --all\n \ttest_path_is_file pager_used\n '\n \n  test_expect_success TTY 'format-patch --stdout pagination can be disabled' '\n \trm -f pager_used &&\n-\t(\n-\t\tGIT_PAGER=\"wc >pager_used\" &&\n-\t\texport GIT_PAGER &&\n-\t\ttest_terminal git --no-pager format-patch --stdout --all &&\n-\t\ttest_terminal git -c \"pager.format-patch=false\" format-patch --stdout --all\n-\t) &&\n+\ttest_terminal env GIT_PAGER=\"wc >pager_used\" git --no-pager format-patch --stdout --all &&\n+\ttest_terminal env GIT_PAGER=\"wc >pager_used\" git -c \"pager.format-patch=false\" format-patch --stdout --all\n \ttest_path_is_missing pager_used &&\n \ttest_path_is_missing .git/pager_used\n '\ndiff --git a/t/t5305-include-tag.sh b/t/t5305-include-tag.sh\nindex b061864..21517c7 100755\n--- a/t/t5305-include-tag.sh\n+++ b/t/t5305-include-tag.sh\n@@ -45,9 +45,7 @@ test_expect_success 'unpack objects' '\n test_expect_success 'check unpacked result (have commit, no tag)' '\n \tgit rev-list --objects $commit >list.expect &&\n \t(\n-\t\tGIT_DIR=clone.git &&\n-\t\texport GIT_DIR &&\n-\t\ttest_must_fail git cat-file -e $tag &&\n+\t\ttest_must_fail env GIT_DIR=clone.git git cat-file -e $tag &&\n \t\tgit rev-list --objects $commit\n \t) >list.actual &&\n \ttest_cmp list.expect list.actual\ndiff --git a/t/t5602-clone-remote-exec.sh b/t/t5602-clone-remote-exec.sh\nindex 3f353d9..70ab206 100755\n--- a/t/t5602-clone-remote-exec.sh\n+++ b/t/t5602-clone-remote-exec.sh\n@@ -12,21 +12,14 @@ test_expect_success setup '\n '\n \n test_expect_success 'clone calls git upload-pack unqualified with no -u option' '\n-\t(\n-\t\tGIT_SSH=./not_ssh &&\n-\t\texport GIT_SSH &&\n-\t\ttest_must_fail git clone localhost:/path/to/repo junk\n-\t) &&\n+\ttest_must_fail env GIT_SSH=./not_ssh git clone localhost:/path/to/repo junk\n \techo \"localhost git-upload-pack '\\''/path/to/repo'\\''\" >expected &&\n \ttest_cmp expected not_ssh_output\n '\n \n test_expect_success 'clone calls specified git upload-pack with -u option' '\n-\t(\n-\t\tGIT_SSH=./not_ssh &&\n-\t\texport GIT_SSH &&\n-\t\ttest_must_fail git clone -u ./something/bin/git-upload-pack localhost:/path/to/repo junk\n-\t) &&\n+\ttest_must_fail env GIT_SSH=./not_ssh \\\n+\t git clone -u ./something/bin/git-upload-pack localhost:/path/to/repo junk\n \techo \"localhost ./something/bin/git-upload-pack '\\''/path/to/repo'\\''\" >expected &&\n \ttest_cmp expected not_ssh_output\n '\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex 613f69a..ca19838 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -218,10 +218,8 @@ test_expect_success 'proper failure checks for fetching' '\n '\n \n test_expect_success 'proper failure checks for pushing' '\n-\t(GIT_REMOTE_TESTGIT_FAILURE=1 &&\n-\texport GIT_REMOTE_TESTGIT_FAILURE &&\n-\tcd local &&\n-\ttest_must_fail git push --all\n+\t(cd local &&\n+\ttest_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git push --all\n \t)\n '\n \ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex 9874403..b43fb67 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -191,8 +191,7 @@ test_expect_success '%C(auto) respects --no-color' '\n \n test_expect_success TTY '%C(auto) respects --color=auto (stdout is tty)' '\n \t(\n-\t\tTERM=vt100 && export TERM &&\n-\t\ttest_terminal \\\n+\t\ttest_terminal env TERM=vt100 \\\n \t\t\tgit log --format=$AUTO_COLOR -1 --color=auto >actual &&\n \t\thas_color actual\n \t)\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex b9365b4..8748ace 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -146,11 +146,7 @@ test_expect_success 'no color when stdout is a regular file' '\n test_expect_success TTY 'color when writing to a pager' '\n \trm -f paginated.out &&\n \ttest_config color.ui auto &&\n-\t(\n-\t\tTERM=vt100 &&\n-\t\texport TERM &&\n-\t\ttest_terminal git log\n-\t) &&\n+\ttest_terminal env TERM=vt100 git log &&\n \tcolorful paginated.out\n '\n \n@@ -158,11 +154,7 @@ test_expect_success TTY 'colors are suppressed by color.pager' '\n \trm -f paginated.out &&\n \ttest_config color.ui auto &&\n \ttest_config color.pager false &&\n-\t(\n-\t\tTERM=vt100 &&\n-\t\texport TERM &&\n-\t\ttest_terminal git log\n-\t) &&\n+\ttest_terminal env TERM=vt100 git log\n \t! colorful paginated.out\n '\n \n@@ -181,11 +173,7 @@ test_expect_success 'color when writing to a file intended for a pager' '\n test_expect_success TTY 'colors are sent to pager for external commands' '\n \ttest_config alias.externallog \"!git log\" &&\n \ttest_config color.ui auto &&\n-\t(\n-\t\tTERM=vt100 &&\n-\t\texport TERM &&\n-\t\ttest_terminal git -p externallog\n-\t) &&\n+\ttest_terminal env TERM=vt100 git -p externallog\n \tcolorful paginated.out\n '\n \n-- \n1.7.9\n"},{"id":"236871","messageId":"CAPig+cToDs9OtWUC2i7Qj9ysJppo9V5Db8SnnPO3RMdPkdTLWw@mail.gmail.com","threadId":"36186","inReplyTo":"1394955601-18829-1-git-send-email-unsignedzero@gmail.com","subject":"Re: [PATCH] Removed subshell invocations in many of the tests when possible","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-17T10:19:16Z","receivedAt":"2014-03-17T10:19:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Thanks for the submission. Comments below to give you a taste of the\nGit review process..\n\nOn Sun, Mar 16, 2014 at 3:40 AM, David Tran <unsignedzero@gmail.com> wrote:\n> Subject: Removed subshell invocations in many of the tests when possible\n\nUse imperative mode: \"remove\" rather than \"removed\"\n\n\"many of the tests\" says little and doesn't add anything useful that\nis not already shown in the diffstat (below).\n\n\"when possible\" is implied: no need to state it.\n\nYou might rewrite as:\n\n  Subject: tests: set temp variables via 'env' rather than subshell\n\nIt also would be a very good idea at this point in the commit message\nto explain why those subshells were used rather than the more\nidiomatic \"FOO=BAR command\". Explain that that form leaks FOO=BAR into\nthe surrounding context in POSIX shells when 'command' is a shell\nfunction. Perhaps reference this [1] email thread or some other\nmeaningful source.\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/243933/focus=243967\n\n> I am David Tran a graduating CS/Math senior from Sonoma State University,\n> United States. I would like to work with git for GSoC'14, specifically the line\n> options for git rebase --interactive. I have used git for a few years and know\n> how destructive but important rebase is to git. I have created a few shell\n> scripts here and there to make life using bash/zsh easier. I would like to\n> apply these skills and work with the best.\n>\n> I've submitted my application yesterday and my patch didn't send correctly.\n\nThis email commentary should be placed below the \"---\" line following\nyour sign-off. It would not make sense as part of the actual commit\nmessage. (Think about someone reading the commit message months or\nyears from now.)\n\nThe patch itself has some problems. See comments below.\n\n> Signed-off-by: David Tran <unsignedzero@gmail.com>\n> ---\n>  t/t1300-repo-config.sh        |   17 +++---------\n>  t/t1510-repo-setup.sh         |    4 +--\n>  t/t3200-branch.sh             |   12 +-------\n>  t/t3301-notes.sh              |   24 ++++++------------\n>  t/t3404-rebase-interactive.sh |   55 +++++++++--------------------------------\n>  t/t3413-rebase-hook.sh        |    6 +---\n>  t/t4014-format-patch.sh       |   14 ++--------\n>  t/t5305-include-tag.sh        |    4 +--\n>  t/t5602-clone-remote-exec.sh  |   13 ++-------\n>  t/t5801-remote-helpers.sh     |    6 +---\n>  t/t6006-rev-list-format.sh    |    3 +-\n>  t/t7006-pager.sh              |   18 ++-----------\n>  12 files changed, 41 insertions(+), 135 deletions(-)\n>\n> diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\n> index c9c426c..3e3f77b 100755\n> --- a/t/t1300-repo-config.sh\n> +++ b/t/t1300-repo-config.sh\n> @@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' '\n>  '\n>\n>  test_expect_success 'nonexistent configuration' '\n> -       (\n> -               GIT_CONFIG=doesnotexist &&\n> -               export GIT_CONFIG &&\n> -               test_must_fail git config --list &&\n> -               test_must_fail git config test.xyzzy\n> -       )\n> +       test_must_fail env GIT_CONFIG=doesnotexist git config --list &&\n> +       test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy\n>  '\n>\n>  test_expect_success SYMLINKS 'symlink to nonexistent configuration' '\n>         ln -s doesnotexist linktonada &&\n>         ln -s linktonada linktolinktonada &&\n> -       (\n> -               GIT_CONFIG=linktonada &&\n> -               export GIT_CONFIG &&\n> -               test_must_fail git config --list &&\n> -               GIT_CONFIG=linktolinktonada &&\n> -               test_must_fail git config --list\n> -       )\n> +       test_must_fail env GIT_CONFIG=linktonada git config --list &&\n> +       test_must_fail env GIT_CONFIG=linktolinktonada git config --list\n>  '\n>\n>  test_expect_success 'check split_cmdline return' \"\n> diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh\n> index cf2ee78..d8025be 100755\n> --- a/t/t1510-repo-setup.sh\n> +++ b/t/t1510-repo-setup.sh\n> @@ -777,9 +777,7 @@ test_expect_success '#30: core.worktree and core.bare conflict (gitfile version)\n>         setup_repo 30 \"$here/30\" gitfile true &&\n>         (\n>                 cd 30 &&\n> -               GIT_DIR=.git &&\n> -               export GIT_DIR &&\n> -               test_must_fail git symbolic-ref HEAD 2>result\n> +               test_must_fail env GIT_DIR='.git' git symbolic-ref HEAD 2>result\n\nWhy quote GIT_DIR='.git' when it wasn't quoted in the original? Also,\nnote that this entire test code is inside single quotes already.\n\n>         ) &&\n>         grep \"core.bare and core.worktree\" 30/result\n>  '\n> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n> index fcdb867..d45e95c 100755\n> --- a/t/t3200-branch.sh\n> +++ b/t/t3200-branch.sh\n> @@ -849,11 +849,7 @@ test_expect_success 'detect typo in branch name when using --edit-description' '\n>         write_script editor <<-\\EOF &&\n>                 echo \"New contents\" >\"$1\"\n>         EOF\n> -       (\n> -               EDITOR=./editor &&\n> -               export EDITOR &&\n> -               test_must_fail git branch --edit-description no-such-branch\n> -       )\n> +       test_must_fail env EDITOR=./editor git branch --edit-description no-such-branch\n>  '\n>\n>  test_expect_success 'refuse --edit-description on unborn branch for now' '\n> @@ -861,11 +857,7 @@ test_expect_success 'refuse --edit-description on unborn branch for now' '\n>                 echo \"New contents\" >\"$1\"\n>         EOF\n>         git checkout --orphan unborn &&\n> -       (\n> -               EDITOR=./editor &&\n> -               export EDITOR &&\n> -               test_must_fail git branch --edit-description\n> -       )\n> +       test_must_fail env EDITOR=./editor git branch --edit-description\n>  '\n>\n>  test_expect_success '--merged catches invalid object names' '\n> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\n> index 3bb79a4..ca1fea9 100755\n> --- a/t/t3301-notes.sh\n> +++ b/t/t3301-notes.sh\n> @@ -17,7 +17,7 @@ GIT_EDITOR=./fake_editor.sh\n>  export GIT_EDITOR\n>\n>  test_expect_success 'cannot annotate non-existing HEAD' '\n> -       (MSG=3 && export MSG && test_must_fail git notes add)\n> +       test_must_fail env MSG=3 git notes add\n>  '\n>\n>  test_expect_success setup '\n> @@ -32,22 +32,18 @@ test_expect_success setup '\n>  '\n>\n>  test_expect_success 'need valid notes ref' '\n> -       (MSG=1 GIT_NOTES_REF=/ && export MSG GIT_NOTES_REF &&\n> -        test_must_fail git notes add) &&\n> -       (MSG=2 GIT_NOTES_REF=/ && export MSG GIT_NOTES_REF &&\n> -        test_must_fail git notes show)\n> +       test_must_fail env MSG=1 env GIT_NOTES_REF=/ git notes show &&\n> +       test_must_fail env MSG=2 env GIT_NOTES_REF=/ git notes show\n\nA single env invocation accepts multiple VAR=VAL arguments, so it\nshould not be necessary to make env invoke env in order to set\nmultiple arguments. The same comment applies to all other such\ninstances below this point.\n\n>  '\n>\n>  test_expect_success 'refusing to add notes in refs/heads/' '\n> -       (MSG=1 GIT_NOTES_REF=refs/heads/bogus &&\n> -        export MSG GIT_NOTES_REF &&\n> -        test_must_fail git notes add)\n> +       test_must_fail env MSG=1 env GIT_NOTES_REF=refs/heads/bogus \\\n> +        git notes add\n>  '\n>\n>  test_expect_success 'refusing to edit notes in refs/remotes/' '\n> -       (MSG=1 GIT_NOTES_REF=refs/remotes/bogus &&\n> -        export MSG GIT_NOTES_REF &&\n> -        test_must_fail git notes edit)\n> +       test_must_fail env MSG=1 env GIT_NOTES_REF=refs/heads/bogus \\\n> +        git notes edit\n>  '\n>\n>  # 1 indicates caught gracefully by die, 128 means git-show barked\n> @@ -865,11 +861,7 @@ test_expect_success 'create note from non-existing note with \"git notes add -c\"\n>         git add a10 &&\n>         test_tick &&\n>         git commit -m 10th &&\n> -       (\n> -               MSG=\"yet another note\" &&\n> -               export MSG &&\n> -               test_must_fail git notes add -c deadbeef\n> -       ) &&\n> +       test_must_fail env MSG=\"yet another note\" git notes add -c deadbeef\n\nBroken &&-chain.\n\n>         test_must_fail git notes list HEAD\n>  '\n>\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 50e22b1..842a47a 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -104,9 +104,7 @@ test_expect_success 'rebase -i with the exec command checks tree cleanness' '\n>         git checkout master &&\n>         (\n>         set_fake_editor &&\n> -       FAKE_LINES=\"exec_echo_foo_>file1 1\" &&\n> -       export FAKE_LINES &&\n> -       test_must_fail git rebase -i HEAD^\n> +       test_must_fail env FAKE_LINES=\"exec_echo_foo_>file1 1\" git rebase -i HEAD^\n>         ) &&\n>         test_cmp_rev master^ HEAD &&\n>         git reset --hard &&\n> @@ -118,9 +116,8 @@ test_expect_success 'rebase -i with exec of inexistent command' '\n>         test_when_finished \"git rebase --abort\" &&\n>         (\n>         set_fake_editor &&\n> -       FAKE_LINES=\"exec_this-command-does-not-exist 1\" &&\n> -       export FAKE_LINES &&\n> -       test_must_fail git rebase -i HEAD^ >actual 2>&1\n> +       test_must_fail env FAKE_LINES=\"exec_this-command-does-not-exist 1\" \\\n> +       git rebase -i HEAD^ >actual 2>&1\n>         ) &&\n>         ! grep \"Maybe git-rebase is broken\" actual\n>  '\n> @@ -375,11 +372,7 @@ test_expect_success 'commit message used after conflict' '\n>         git checkout -b conflict-fixup conflict-branch &&\n>         base=$(git rev-parse HEAD~4) &&\n>         set_fake_editor &&\n> -       (\n> -               FAKE_LINES=\"1 fixup 3 fixup 4\" &&\n> -               export FAKE_LINES &&\n> -               test_must_fail git rebase -i $base\n> -       ) &&\n> +       test_must_fail env FAKE_LINES=\"1 fixup 3 fixup 4\" git rebase -i $base &&\n>         echo three > conflict &&\n>         git add conflict &&\n>         FAKE_COMMIT_AMEND=\"ONCE\" EXPECT_HEADER_COUNT=2 \\\n> @@ -394,11 +387,7 @@ test_expect_success 'commit message retained after conflict' '\n>         git checkout -b conflict-squash conflict-branch &&\n>         base=$(git rev-parse HEAD~4) &&\n>         set_fake_editor &&\n> -       (\n> -               FAKE_LINES=\"1 fixup 3 squash 4\" &&\n> -               export FAKE_LINES &&\n> -               test_must_fail git rebase -i $base\n> -       ) &&\n> +       test_must_fail env FAKE_LINES=\"1 fixup 3 squash 4\" git rebase -i $base &&\n>         echo three > conflict &&\n>         git add conflict &&\n>         FAKE_COMMIT_AMEND=\"TWICE\" EXPECT_HEADER_COUNT=2 \\\n> @@ -469,11 +458,7 @@ test_expect_success 'interrupted squash works as expected' '\n>         git checkout -b interrupted-squash conflict-branch &&\n>         one=$(git rev-parse HEAD~3) &&\n>         set_fake_editor &&\n> -       (\n> -               FAKE_LINES=\"1 squash 3 2\" &&\n> -               export FAKE_LINES &&\n> -               test_must_fail git rebase -i HEAD~3\n> -       ) &&\n> +       test_must_fail env FAKE_LINES=\"1 squash 3 2\" git rebase -i HEAD~3 &&\n>         (echo one; echo two; echo four) > conflict &&\n>         git add conflict &&\n>         test_must_fail git rebase --continue &&\n> @@ -487,11 +472,7 @@ test_expect_success 'interrupted squash works as expected (case 2)' '\n>         git checkout -b interrupted-squash2 conflict-branch &&\n>         one=$(git rev-parse HEAD~3) &&\n>         set_fake_editor &&\n> -       (\n> -               FAKE_LINES=\"3 squash 1 2\" &&\n> -               export FAKE_LINES &&\n> -               test_must_fail git rebase -i HEAD~3\n> -       ) &&\n> +       test_must_fail env FAKE_LINES=\"3 squash 1 2\" git rebase -i HEAD~3 &&\n>         (echo one; echo four) > conflict &&\n>         git add conflict &&\n>         test_must_fail git rebase --continue &&\n> @@ -529,9 +510,7 @@ test_expect_success 'aborted --continue does not squash commits after \"edit\"' '\n>         echo \"edited again\" > file7 &&\n>         git add file7 &&\n>         (\n> -               FAKE_COMMIT_MESSAGE=\" \" &&\n> -               export FAKE_COMMIT_MESSAGE &&\n> -               test_must_fail git rebase --continue\n> +               test_must_fail env FAKE_COMMIT_MESSAGE=\" \" git rebase --continue\n>         ) &&\n\nDon't you want to get rid of the subshell here?\n\n>         test $old = $(git rev-parse HEAD) &&\n>         git rebase --abort\n> @@ -548,9 +527,7 @@ test_expect_success 'auto-amend only edited commits after \"edit\"' '\n>         git add file7 &&\n>         test_tick &&\n>         (\n> -               FAKE_COMMIT_MESSAGE=\"and again\" &&\n> -               export FAKE_COMMIT_MESSAGE &&\n> -               test_must_fail git rebase --continue\n> +               test_must_fail env FAKE_COMMIT_MESSAGE=\"and again\" git rebase --continue\n>         ) &&\n\nAnd here?\n\n>         git rebase --abort\n>  '\n> @@ -560,9 +537,7 @@ test_expect_success 'clean error after failed \"exec\"' '\n>         test_when_finished \"git rebase --abort || :\" &&\n>         set_fake_editor &&\n>         (\n> -               FAKE_LINES=\"1 exec_false\" &&\n> -               export FAKE_LINES &&\n> -               test_must_fail git rebase -i HEAD^\n> +               test_must_fail env FAKE_LINES=\"1 exec_false\" git rebase -i HEAD^\n>         ) &&\n\nAnd here?\n\n>         echo \"edited again\" > file7 &&\n>         git add file7 &&\n> @@ -949,9 +924,7 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '\n>         git checkout B &&\n>         (\n>                 set_fake_editor &&\n> -               FAKE_LINES=\"2\" &&\n> -               export FAKE_LINES &&\n> -               test_must_fail git rebase -i --root\n> +               test_must_fail env FAKE_LINES=\"2\" git rebase -i --root\n>         ) &&\n\nAnd maybe here (or does this test or later tests break if\nset_fake_editor is taken out of the subshell)?\n\n>         git cat-file commit HEAD | grep \"^tree 4b825dc642cb\" &&\n>         git rebase --abort\n> @@ -1042,11 +1015,7 @@ test_expect_success 'rebase -i error on commits with \\ in message' '\n>         test_when_finished \"git rebase --abort; git reset --hard $current_head; rm -f error\" &&\n>         test_commit TO-REMOVE will-conflict old-content &&\n>         test_commit \"\\temp\" will-conflict new-content dummy &&\n> -       (\n> -       EDITOR=true &&\n> -       export EDITOR &&\n> -       test_must_fail git rebase -i HEAD^ --onto HEAD^^ 2>error\n> -       ) &&\n> +       test_must_fail env EDITOR=true git rebase -i HEAD^ --onto HEAD^^ 2>error\n\nBroken &&-chain.\n\n>         test_expect_code 1 grep  \"      emp\" error\n>  '\n>\n> diff --git a/t/t3413-rebase-hook.sh b/t/t3413-rebase-hook.sh\n> index 098b755..33560a1 100755\n> --- a/t/t3413-rebase-hook.sh\n> +++ b/t/t3413-rebase-hook.sh\n> @@ -118,11 +118,7 @@ test_expect_success 'pre-rebase hook stops rebase (1)' '\n>  test_expect_success 'pre-rebase hook stops rebase (2)' '\n>         git checkout test &&\n>         git reset --hard side &&\n> -       (\n> -               EDITOR=:\n> -               export EDITOR\n> -               test_must_fail git rebase -i master\n> -       ) &&\n> +       test_must_fail env EDITOR=: git rebase -i master\n\nBroken &&-chain.\n\n>         test \"z$(git symbolic-ref HEAD)\" = zrefs/heads/test &&\n>         test 0 = $(git rev-list HEAD...side | wc -l)\n>  '\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index 73194b2..4a4b943 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -764,22 +764,14 @@ test_expect_success 'format-patch --signature=\"\" suppresses signatures' '\n>\n>  test_expect_success TTY 'format-patch --stdout paginates' '\n>         rm -f pager_used &&\n> -       (\n> -               GIT_PAGER=\"wc >pager_used\" &&\n> -               export GIT_PAGER &&\n> -               test_terminal git format-patch --stdout --all\n> -       ) &&\n> +       test_terminal env GIT_PAGER=\"wc >pager_used\" git format-patch --stdout --all\n\nBroken &&-chain.\n\n>         test_path_is_file pager_used\n>  '\n>\n>   test_expect_success TTY 'format-patch --stdout pagination can be disabled' '\n>         rm -f pager_used &&\n> -       (\n> -               GIT_PAGER=\"wc >pager_used\" &&\n> -               export GIT_PAGER &&\n> -               test_terminal git --no-pager format-patch --stdout --all &&\n> -               test_terminal git -c \"pager.format-patch=false\" format-patch --stdout --all\n> -       ) &&\n> +       test_terminal env GIT_PAGER=\"wc >pager_used\" git --no-pager format-patch --stdout --all &&\n> +       test_terminal env GIT_PAGER=\"wc >pager_used\" git -c \"pager.format-patch=false\" format-patch --stdout --all\n\nBroken &&-chain.\n\n>         test_path_is_missing pager_used &&\n>         test_path_is_missing .git/pager_used\n>  '\n> diff --git a/t/t5305-include-tag.sh b/t/t5305-include-tag.sh\n> index b061864..21517c7 100755\n> --- a/t/t5305-include-tag.sh\n> +++ b/t/t5305-include-tag.sh\n> @@ -45,9 +45,7 @@ test_expect_success 'unpack objects' '\n>  test_expect_success 'check unpacked result (have commit, no tag)' '\n>         git rev-list --objects $commit >list.expect &&\n>         (\n> -               GIT_DIR=clone.git &&\n> -               export GIT_DIR &&\n> -               test_must_fail git cat-file -e $tag &&\n> +               test_must_fail env GIT_DIR=clone.git git cat-file -e $tag &&\n>                 git rev-list --objects $commit\n>         ) >list.actual &&\n>         test_cmp list.expect list.actual\n> diff --git a/t/t5602-clone-remote-exec.sh b/t/t5602-clone-remote-exec.sh\n> index 3f353d9..70ab206 100755\n> --- a/t/t5602-clone-remote-exec.sh\n> +++ b/t/t5602-clone-remote-exec.sh\n> @@ -12,21 +12,14 @@ test_expect_success setup '\n>  '\n>\n>  test_expect_success 'clone calls git upload-pack unqualified with no -u option' '\n> -       (\n> -               GIT_SSH=./not_ssh &&\n> -               export GIT_SSH &&\n> -               test_must_fail git clone localhost:/path/to/repo junk\n> -       ) &&\n> +       test_must_fail env GIT_SSH=./not_ssh git clone localhost:/path/to/repo junk\n\nBroken &&-chain.\n\n>         echo \"localhost git-upload-pack '\\''/path/to/repo'\\''\" >expected &&\n>         test_cmp expected not_ssh_output\n>  '\n>\n>  test_expect_success 'clone calls specified git upload-pack with -u option' '\n> -       (\n> -               GIT_SSH=./not_ssh &&\n> -               export GIT_SSH &&\n> -               test_must_fail git clone -u ./something/bin/git-upload-pack localhost:/path/to/repo junk\n> -       ) &&\n> +       test_must_fail env GIT_SSH=./not_ssh \\\n> +        git clone -u ./something/bin/git-upload-pack localhost:/path/to/repo junk\n\nBroken &&-chain.\n\nStrange indentation on the continuation line. Indent with tab.\n\n>         echo \"localhost ./something/bin/git-upload-pack '\\''/path/to/repo'\\''\" >expected &&\n>         test_cmp expected not_ssh_output\n>  '\n> diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\n> index 613f69a..ca19838 100755\n> --- a/t/t5801-remote-helpers.sh\n> +++ b/t/t5801-remote-helpers.sh\n> @@ -218,10 +218,8 @@ test_expect_success 'proper failure checks for fetching' '\n>  '\n>\n>  test_expect_success 'proper failure checks for pushing' '\n> -       (GIT_REMOTE_TESTGIT_FAILURE=1 &&\n> -       export GIT_REMOTE_TESTGIT_FAILURE &&\n> -       cd local &&\n> -       test_must_fail git push --all\n> +       (cd local &&\n> +       test_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git push --all\n>         )\n>  '\n>\n> diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\n> index 9874403..b43fb67 100755\n> --- a/t/t6006-rev-list-format.sh\n> +++ b/t/t6006-rev-list-format.sh\n> @@ -191,8 +191,7 @@ test_expect_success '%C(auto) respects --no-color' '\n>\n>  test_expect_success TTY '%C(auto) respects --color=auto (stdout is tty)' '\n>         (\n> -               TERM=vt100 && export TERM &&\n> -               test_terminal \\\n> +               test_terminal env TERM=vt100 \\\n>                         git log --format=$AUTO_COLOR -1 --color=auto >actual &&\n>                 has_color actual\n>         )\n\nIs this subshell still necessary? Does has_color rely upon the value of TERM?\n\n> diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\n> index b9365b4..8748ace 100755\n> --- a/t/t7006-pager.sh\n> +++ b/t/t7006-pager.sh\n> @@ -146,11 +146,7 @@ test_expect_success 'no color when stdout is a regular file' '\n>  test_expect_success TTY 'color when writing to a pager' '\n>         rm -f paginated.out &&\n>         test_config color.ui auto &&\n> -       (\n> -               TERM=vt100 &&\n> -               export TERM &&\n> -               test_terminal git log\n> -       ) &&\n> +       test_terminal env TERM=vt100 git log &&\n>         colorful paginated.out\n>  '\n>\n> @@ -158,11 +154,7 @@ test_expect_success TTY 'colors are suppressed by color.pager' '\n>         rm -f paginated.out &&\n>         test_config color.ui auto &&\n>         test_config color.pager false &&\n> -       (\n> -               TERM=vt100 &&\n> -               export TERM &&\n> -               test_terminal git log\n> -       ) &&\n> +       test_terminal env TERM=vt100 git log\n\nBroken &&-chain.\n\n>         ! colorful paginated.out\n>  '\n>\n> @@ -181,11 +173,7 @@ test_expect_success 'color when writing to a file intended for a pager' '\n>  test_expect_success TTY 'colors are sent to pager for external commands' '\n>         test_config alias.externallog \"!git log\" &&\n>         test_config color.ui auto &&\n> -       (\n> -               TERM=vt100 &&\n> -               export TERM &&\n> -               test_terminal git -p externallog\n> -       ) &&\n> +       test_terminal env TERM=vt100 git -p externallog\n\nBroken &&-chain.\n\n>         colorful paginated.out\n>  '\n>\n> --\n> 1.7.9\n"}]}