tests: use "env" to run commands with temporary env-var settings
PATCH v3 1/1 tests: use "env" to run commands with temporary env-var settings
In reply to a message not in the archiveOriginally, we would use "VAR=VAL command" to execute a test command with environment variable(s) only for that command. This does not work for commands that are shell functions (most notably test functions like "test_must_fail"); the result of the assignment is retained and affects later commands.
To avoid this, we assigned and exported the environment variables and run the test(s) in a subshell like this,
( VAR=VAL && export VAR test_must_fail git command to be tested )
Using the "env" utility, we should be able to say
test_must_fail git command to be tested
which is much short and easier to read.
---
Quoted 3 lines
Quoted 2 lines
Let's see if I replied correctly with send-email. Retrying this again. How do I 'reply' to a thread using send-email?
Look for --in-reply-to option in "man git-send-email".
Git prompts me for this but I don't know what to type. I tried typing just the id, like 244379 and that didn't work. I guess I'll try [email protected]? Google didn't bring many results on how to use it but many results of what it is and its goal.
Quoted 6 lines
Isn't
GIT_CONFIGhere another way of saying:test_must_fail git config -f doesnotexist --listPerhaps that is shorter and more readable still (and there are a few similar cases in this patch.
I'll ignore this for now. If needed I can make another patch to resolve this.
Hopefully this should be all of it.
I am David Tran a graduating CS/Math senior from Sonoma State University, United States. I would like to work with git for GSoC'14, specifically the line options for git rebase --interactive [1][2]. I have used git for a few years and know how destructive but important rebase is to git. I have created a few shell scripts here and there to make life using bash/zsh easier. I would like to apply these skills and work with the best.
Github: unsignedzero
[1]: http://thread.gmane.org/gmane.comp.version-control.git/243933/focus=243967
[2]: http://thread.gmane.org/gmane.comp.version-control.git/242701
--- t/t1300-repo-config.sh | 17 ++-------- t/t1510-repo-setup.sh | 4 +-- t/t3200-branch.sh | 12 +------ t/t3301-notes.sh | 22 +++--------- t/t3404-rebase-interactive.sh | 69 ++++++++--------------------------------- t/t3413-rebase-hook.sh | 6 +--- t/t4014-format-patch.sh | 14 ++------ t/t5305-include-tag.sh | 4 +-- t/t5602-clone-remote-exec.sh | 13 ++------ t/t5801-remote-helpers.sh | 6 +-- t/t6006-rev-list-format.sh | 9 ++--- t/t7006-pager.sh | 18 ++--------- 12 files changed, 42 insertions(+), 152 deletions(-)
t/t1300-repo-config.sh +4 -13
@@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' ' ' test_expect_success 'nonexistent configuration' ' - ( - GIT_CONFIG=doesnotexist && - export GIT_CONFIG && - test_must_fail git config --list && - test_must_fail git config test.xyzzy - ) + test_must_fail env GIT_CONFIG=doesnotexist git config --list && + test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy ' test_expect_success SYMLINKS 'symlink to nonexistent configuration' ' ln -s doesnotexist linktonada && ln -s linktonada linktolinktonada && - ( - GIT_CONFIG=linktonada && - export GIT_CONFIG && - test_must_fail git config --list && - GIT_CONFIG=linktolinktonada && - test_must_fail git config --list - ) + test_must_fail env GIT_CONFIG=linktonada git config --list && + test_must_fail env GIT_CONFIG=linktolinktonada git config --list ' test_expect_success 'check split_cmdline return' "
t/t1510-repo-setup.sh +1 -3
@@ -777,9 +777,7 @@ test_expect_success '#30: core.worktree and core.bare conflict (gitfile version) setup_repo 30 "$here/30" gitfile true && ( cd 30 && - GIT_DIR=.git && - export GIT_DIR && - test_must_fail git symbolic-ref HEAD 2>result + test_must_fail env GIT_DIR=.git git symbolic-ref HEAD 2>result ) && grep "core.bare and core.worktree" 30/result '
t/t3200-branch.sh +2 -10
@@ -849,11 +849,7 @@ test_expect_success 'detect typo in branch name when using --edit-description' ' write_script editor <<-\EOF && echo "New contents" >"$1" EOF - ( - EDITOR=./editor && - export EDITOR && - test_must_fail git branch --edit-description no-such-branch - ) + test_must_fail env EDITOR=./editor git branch --edit-description no-such-branch ' test_expect_success 'refuse --edit-description on unborn branch for now' '
@@ -861,11 +857,7 @@ test_expect_success 'refuse --edit-description on unborn branch for now' ' echo "New contents" >"$1" EOF git checkout --orphan unborn && - ( - EDITOR=./editor && - export EDITOR && - test_must_fail git branch --edit-description - ) + test_must_fail env EDITOR=./editor git branch --edit-description ' test_expect_success '--merged catches invalid object names' '
t/t3301-notes.sh +6 -16
@@ -17,7 +17,7 @@ GIT_EDITOR=./fake_editor.sh export GIT_EDITOR test_expect_success 'cannot annotate non-existing HEAD' ' - (MSG=3 && export MSG && test_must_fail git notes add) + test_must_fail env MSG=3 git notes add ' test_expect_success setup '
@@ -32,22 +32,16 @@ test_expect_success setup ' ' test_expect_success 'need valid notes ref' ' - (MSG=1 GIT_NOTES_REF=/ && export MSG GIT_NOTES_REF && - test_must_fail git notes add) && - (MSG=2 GIT_NOTES_REF=/ && export MSG GIT_NOTES_REF && - test_must_fail git notes show) + test_must_fail env MSG=1 GIT_NOTES_REF=/ git notes show && + test_must_fail env MSG=2 GIT_NOTES_REF=/ git notes show ' test_expect_success 'refusing to add notes in refs/heads/' ' - (MSG=1 GIT_NOTES_REF=refs/heads/bogus && - export MSG GIT_NOTES_REF && - test_must_fail git notes add) + test_must_fail env MSG=1 GIT_NOTES_REF=refs/heads/bogus git notes add ' test_expect_success 'refusing to edit notes in refs/remotes/' ' - (MSG=1 GIT_NOTES_REF=refs/remotes/bogus && - export MSG GIT_NOTES_REF && - test_must_fail git notes edit) + test_must_fail env MSG=1 GIT_NOTES_REF=refs/heads/bogus git notes edit ' # 1 indicates caught gracefully by die, 128 means git-show barked
@@ -865,11 +859,7 @@ test_expect_success 'create note from non-existing note with "git notes add -c" git add a10 && test_tick && git commit -m 10th && - ( - MSG="yet another note" && - export MSG && - test_must_fail git notes add -c deadbeef - ) && + test_must_fail env MSG="yet another note" git notes add -c deadbeef && test_must_fail git notes list HEAD '
t/t3404-rebase-interactive.sh +13 -56
137 lines not on this page · raw message
t/t3413-rebase-hook.sh +1 -5
@@ -118,11 +118,7 @@ test_expect_success 'pre-rebase hook stops rebase (1)' ' test_expect_success 'pre-rebase hook stops rebase (2)' ' git checkout test && git reset --hard side && - ( - EDITOR=: - export EDITOR - test_must_fail git rebase -i master - ) && + test_must_fail env EDITOR=: git rebase -i master && test "z$(git symbolic-ref HEAD)" = zrefs/heads/test && test 0 = $(git rev-list HEAD...side | wc -l) '
t/t4014-format-patch.sh +3 -11
@@ -764,22 +764,14 @@ test_expect_success 'format-patch --signature="" suppresses signatures' ' test_expect_success TTY 'format-patch --stdout paginates' ' rm -f pager_used && - ( - GIT_PAGER="wc >pager_used" && - export GIT_PAGER && - test_terminal git format-patch --stdout --all - ) && + test_terminal env GIT_PAGER="wc >pager_used" git format-patch --stdout --all && test_path_is_file pager_used ' test_expect_success TTY 'format-patch --stdout pagination can be disabled' ' rm -f pager_used && - ( - GIT_PAGER="wc >pager_used" && - export GIT_PAGER && - test_terminal git --no-pager format-patch --stdout --all && - test_terminal git -c "pager.format-patch=false" format-patch --stdout --all - ) && + test_terminal env GIT_PAGER="wc >pager_used" git --no-pager format-patch --stdout --all && + test_terminal env GIT_PAGER="wc >pager_used" git -c "pager.format-patch=false" format-patch --stdout --all && test_path_is_missing pager_used && test_path_is_missing .git/pager_used '
t/t5305-include-tag.sh +1 -3
@@ -45,9 +45,7 @@ test_expect_success 'unpack objects' ' test_expect_success 'check unpacked result (have commit, no tag)' ' git rev-list --objects $commit >list.expect && ( - GIT_DIR=clone.git && - export GIT_DIR && - test_must_fail git cat-file -e $tag && + test_must_fail env GIT_DIR=clone.git git cat-file -e $tag && git rev-list --objects $commit ) >list.actual && test_cmp list.expect list.actual
t/t5602-clone-remote-exec.sh +3 -10
@@ -12,21 +12,14 @@ test_expect_success setup ' ' test_expect_success 'clone calls git upload-pack unqualified with no -u option' ' - ( - GIT_SSH=./not_ssh && - export GIT_SSH && - test_must_fail git clone localhost:/path/to/repo junk - ) && + test_must_fail env GIT_SSH=./not_ssh git clone localhost:/path/to/repo junk && echo "localhost git-upload-pack '\''/path/to/repo'\''" >expected && test_cmp expected not_ssh_output ' test_expect_success 'clone calls specified git upload-pack with -u option' ' - ( - GIT_SSH=./not_ssh && - export GIT_SSH && - test_must_fail git clone -u ./something/bin/git-upload-pack localhost:/path/to/repo junk - ) && + test_must_fail env GIT_SSH=./not_ssh \ + git clone -u ./something/bin/git-upload-pack localhost:/path/to/repo junk && echo "localhost ./something/bin/git-upload-pack '\''/path/to/repo'\''" >expected && test_cmp expected not_ssh_output '
t/t5801-remote-helpers.sh +2 -4
@@ -218,10 +218,8 @@ test_expect_success 'proper failure checks for fetching' ' ' test_expect_success 'proper failure checks for pushing' ' - (GIT_REMOTE_TESTGIT_FAILURE=1 && - export GIT_REMOTE_TESTGIT_FAILURE && - cd local && - test_must_fail git push --all + (cd local && + test_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git push --all ) '
t/t6006-rev-list-format.sh +3 -6
@@ -190,12 +190,9 @@ test_expect_success '%C(auto) respects --no-color' ' ' test_expect_success TTY '%C(auto) respects --color=auto (stdout is tty)' ' - ( - TERM=vt100 && export TERM && - test_terminal \ - git log --format=$AUTO_COLOR -1 --color=auto >actual && - has_color actual - ) + test_terminal env TERM=vt100 \ + git log --format=$AUTO_COLOR -1 --color=auto >actual && + has_color actual ' test_expect_success '%C(auto) respects --color=auto (stdout not tty)' '
t/t7006-pager.sh +3 -15
@@ -146,11 +146,7 @@ test_expect_success 'no color when stdout is a regular file' ' test_expect_success TTY 'color when writing to a pager' ' rm -f paginated.out && test_config color.ui auto && - ( - TERM=vt100 && - export TERM && - test_terminal git log - ) && + test_terminal env TERM=vt100 git log && colorful paginated.out '
@@ -158,11 +154,7 @@ test_expect_success TTY 'colors are suppressed by color.pager' ' rm -f paginated.out && test_config color.ui auto && test_config color.pager false && - ( - TERM=vt100 && - export TERM && - test_terminal git log - ) && + test_terminal env TERM=vt100 git log && ! colorful paginated.out '
@@ -181,11 +173,7 @@ test_expect_success 'color when writing to a file intended for a pager' ' test_expect_success TTY 'colors are sent to pager for external commands' ' test_config alias.externallog "!git log" && test_config color.ui auto && - ( - TERM=vt100 && - export TERM && - test_terminal git -p externallog - ) && + test_terminal env TERM=vt100 git -p externallog && colorful paginated.out '
--
1.7.9
Quoted 11 lines · David Tran
Originally, we would use "VAR=VAL command" to execute a test command with environment variable(s) only for that command. This does not work for commands that are shell functions (most notably test functions like "
test_must_fail"); the result of the assignment is retained and affects later commands.To avoid this, we assigned and exported the environment variables and run the test(s) in a subshell like this,
( VAR=VAL && export VAR
Append && to this line.
Quoted 8 lines
test_must_failgit command to be tested
)Using the "env" utility, we should be able to say
test_must_failgit command to be testedwhich is much short and easier to read.
s/short/shorter/
Quoted 5 lines
Signed-off-by David Tran <[email protected]>---
Hopefully this should be all of it.
Much better. I didn't spot any errors in the patch this time around.
One final note for future submissions: As a courtesy to reviewers, explain (below the "---" line) what changed in the current version, and provide a reference to the previous attempt, like this [1].
[1]: http://thread.gmane.org/gmane.comp.version-control.git/244379
Quoted 15 lines
Signed-off-by David Tran <[email protected]>--- t/t1300-repo-config.sh | 17 ++-------- t/t1510-repo-setup.sh | 4 +-- t/t3200-branch.sh | 12 +------ t/t3301-notes.sh | 22 +++--------- t/t3404-rebase-interactive.sh | 69 ++++++++--------------------------------- t/t3413-rebase-hook.sh | 6 +--- t/t4014-format-patch.sh | 14 ++------ t/t5305-include-tag.sh | 4 +-- t/t5602-clone-remote-exec.sh | 13 ++------ t/t5801-remote-helpers.sh | 6 +-- t/t6006-rev-list-format.sh | 9 ++--- t/t7006-pager.sh | 18 ++--------- 12 files changed, 42 insertions(+), 152 deletions(-)
Quoted 14 lines · David Tran
Originally, we would use "VAR=VAL command" to execute a test command with environment variable(s) only for that command. This does not work for commands that are shell functions (most notably test functions like "
test_must_fail"); the result of the assignment is retained and affects later commands.To avoid this, we assigned and exported the environment variables and run the test(s) in a subshell like this,
( VAR=VAL && export VAR test_must_fail git command to be tested )
Quoted 5 lines
Using the "env" utility, we should be able to say
test_must_failgit command to be testedwhich is much short and easier to read.
Looks familiar ;-) but it seems the changes from the original you took it from all look worsening, not improvements, to me.
Quoted 7 lines
Quoted 6 lines
Isn't
GIT_CONFIGhere another way of saying:test_must_fail git config -f doesnotexist --listPerhaps that is shorter and more readable still (and there are a few similar cases in this patch.
I'll ignore this for now. If needed I can make another patch to resolve this.
Yes, I think that is sensible. And it does not have to be done by you.
Quoted 1 line
Hopefully this should be all of it.
Seems to be well done. Thanks.