Re: [PATCH 1/7] tests: subshell indentation stylefix
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:49:30
Jonathan Nieder wrote:
Format the subshells introduced by the previous patch (Several tests: cd inside subshell instead of around, 2010-09-06)
Review of said previous patch follows. Sorry for the confusing diffs.
quoted hunk ↗ jump to hunk
--- a/t/t1020-subdirectory.sh +++ b/t/t1020-subdirectory.sh@@ -27,12 +27,13 @@ test_expect_success 'update-index and ls-files' ' one) echo pass one ;; *) echo bad one; exit 1 ;; esac && - (cd dir && - git update-index --add two && - case "`git ls-files`" in - two) echo pass two ;; - *) echo bad two; exit 1 ;; - esac + ( + cd dir && + git update-index --add two && + case "`git ls-files`" in + two) echo pass two ;; + *) echo bad two; exit 1 ;; + esac ) &&
Trapping the "exit" in a subshell improves behavior. I wonder why the script does not use "false" or "(exit 1)"; the tests outside the subshell have the same problem...
quoted hunk ↗ jump to hunk
--- a/t/t4041-diff-submodule-option.sh +++ b/t/t4041-diff-submodule-option.sh@@ -85,9 +85,10 @@ EOF " commit_file sm1 && -head3=$(cd sm1 && -git reset --hard HEAD~2 >/dev/null && -git rev-parse --verify HEAD | cut -c1-7 +head3=$( + cd sm1 && + git reset --hard HEAD~2 >/dev/null && + git rev-parse --verify HEAD | cut -c1-7
Unrelated: the --verify here does not have much effect on the upstream of a pipeline. Maybe "git log -1 --abbrev=7 --format=%h", or even better, "git log -1 --format=%h" (for DEFAULT_ABBREV) would do the trick.
quoted hunk ↗ jump to hunk
--- a/t/t9100-git-svn-basic.sh +++ b/t/t9100-git-svn-basic.sh@@ -22,16 +22,17 @@ esac test_expect_success \ 'initialize git svn' ' mkdir import && - (cd import && - echo foo > foo && - ln -s foo foo.link - mkdir -p dir/a/b/c/d/e && - echo "deep dir" > dir/a/b/c/d/e/file && - mkdir bar && - echo "zzz" > bar/zzz && - echo "#!/bin/sh" > exec.sh && - chmod +x exec.sh && - svn_cmd import -m "import for git svn" . "$svnrepo" >/dev/null + ( + cd import && + echo foo >foo && + ln -s foo foo.link
Missing SYMLINKS prerequisite?
quoted hunk ↗ jump to hunk
--- a/t/t9107-git-svn-migrate.sh +++ b/t/t9107-git-svn-migrate.sh@@ -6,14 +6,16 @@ test_description='git svn metadata migrations from previous versions' test_expect_success 'setup old-looking metadata' ' cp "$GIT_DIR"/config "$GIT_DIR"/config-old-git-svn && mkdir import && - (cd import && - for i in trunk branches/a branches/b \ - tags/0.1 tags/0.2 tags/0.3; do - mkdir -p $i && \ - echo hello >> $i/README || exit 1 - done && \ + ( + cd import && + for i in trunk branches/a branches/b tags/0.1 tags/0.2 tags/0.3 + do + mkdir -p $i && + echo hello >>$i/README || + exit 1
An "exit" to avoid wasting time after a failing setup test might seem appropriate, but I am happier to see it trapped by the subshell.
quoted hunk ↗ jump to hunk
--- a/t/t9116-git-svn-log.sh +++ b/t/t9116-git-svn-log.sh@@ -8,14 +8,16 @@ test_description='git svn log tests' test_expect_success 'setup repository and import' ' mkdir import && - (cd import && - for i in trunk branches/a branches/b \ - tags/0.1 tags/0.2 tags/0.3; do - mkdir -p $i && \ - echo hello >> $i/README || exit 1 - done && \ + ( + cd import && + for i in trunk branches/a branches/b tags/0.1 tags/0.2 tags/0.3 + do + mkdir -p $i && + echo hello >>$i/README || + exit 1
Likewise.
quoted hunk ↗ jump to hunk
--- a/t/t9119-git-svn-info.sh +++ b/t/t9119-git-svn-info.sh@@ -179,11 +186,13 @@ test_expect_success 'info --url added-directory' ' ' test_expect_success 'info added-symlink-file' " - (cd gitwc && + ( + cd gitwc && ln -s added-file added-symlink-file && git add added-symlink-file ) && - (cd svnwc && + ( + cd svnwc && ln -s added-file added-symlink-file &&
More missing SYMLINKS prerequisites. To summarize, wherever you change behavior, it is improved. Thanks. Reviewed-by: Jonathan Nieder <redacted>