If quoting is generally preferred as a best practice, we could force wc to behave more consistently before we start testing diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 0d93e33..57ed608 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -515,6 +515,14 @@ test_path_is_missing () { fi } +# Some platforms disagree about wc output format. + +SYS_WC=$(which wc) + +wc () { + $SYS_WC $@ | sed -e 's/^ *//' -e 's/ *$//' +} + # test_line_count checks that a file has the number of lines it # ought to. For example: # On Mon, Nov 10, 2014 at 5:40 PM, Johan Herland <johan@xxxxxxxxxxx> wrote: > On Tue, Nov 11, 2014 at 2:19 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: >> On Mon, Nov 10, 2014 at 7:17 PM, Michael Blume <blume.mike@xxxxxxxxx> wrote: >>> the commit modernizing test 3301 >>> (https://github.com/git/git/commit/fbe4f74865acfd) appears to break it >>> on my mac >>> >>> $ ./t3301-notes.sh -v >>> expecting success: >>> MSG=b4 git notes add && >>> test_path_is_missing .git/NOTES_EDITMSG && >>> test "1" = "$(git ls-tree refs/notes/commits | wc -l)" && >>> test "b4" = "$(git notes show)" && >>> git show HEAD^ && >>> test_must_fail git notes show HEAD^ >>> >>> b4 >>> not ok 8 - create notes >> >> This and all other failures are due to the output of 'wc -l', which on >> Mac is "{whitespace}1" rather than just "1" as it is on other >> platforms. fbe4f748 added quotes around the $(... | wc -l) invocation >> which caused the whitespace to be retained. A minimum fix would be to >> drop the quotes surrounding $(). > > Ah, thanks! > > I thought that quoting command output was a good idea in general. Am I > wrong, or is this just one exception to an otherwise good guideline? > > Anyway, here is the minimal diff to remove quoting from the $(... | wc > -l) commands (probably whitespace damaged by GMail - sorry). Junio: > feel free to squash this in, or ask for a re-roll: > > diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh > index cd756ec..2c8abbc 100755 > --- a/t/t3301-notes.sh > +++ b/t/t3301-notes.sh > @@ -52,7 +52,7 @@ test_expect_success 'show non-existent notes entry with %N' ' > test_expect_success 'create notes' ' > MSG=b4 git notes add && > test_path_is_missing .git/NOTES_EDITMSG && > - test "1" = "$(git ls-tree refs/notes/commits | wc -l)" && > + test "1" = $(git ls-tree refs/notes/commits | wc -l) && > test "b4" = "$(git notes show)" && > git show HEAD^ && > test_must_fail git notes show HEAD^ > @@ -75,7 +75,7 @@ test_expect_success 'create reflog entry' ' > test_expect_success 'edit existing notes' ' > MSG=b3 git notes edit && > test_path_is_missing .git/NOTES_EDITMSG && > - test "1" = "$(git ls-tree refs/notes/commits | wc -l)" && > + test "1" = $(git ls-tree refs/notes/commits | wc -l) && > test "b3" = "$(git notes show)" && > git show HEAD^ && > test_must_fail git notes show HEAD^ > @@ -84,7 +84,7 @@ test_expect_success 'edit existing notes' ' > test_expect_success 'cannot "git notes add -m" where notes already exists' ' > test_must_fail git notes add -m "b2" && > test_path_is_missing .git/NOTES_EDITMSG && > - test "1" = "$(git ls-tree refs/notes/commits | wc -l)" && > + test "1" = $(git ls-tree refs/notes/commits | wc -l) && > test "b3" = "$(git notes show)" && > git show HEAD^ && > test_must_fail git notes show HEAD^ > @@ -93,7 +93,7 @@ test_expect_success 'cannot "git notes add -m" where > notes already exists' ' > test_expect_success 'can overwrite existing note with "git notes add -f -m"' ' > git notes add -f -m "b1" && > test_path_is_missing .git/NOTES_EDITMSG && > - test "1" = "$(git ls-tree refs/notes/commits | wc -l)" && > + test "1" = $(git ls-tree refs/notes/commits | wc -l) && > test "b1" = "$(git notes show)" && > git show HEAD^ && > test_must_fail git notes show HEAD^ > @@ -102,7 +102,7 @@ test_expect_success 'can overwrite existing note > with "git notes add -f -m"' ' > test_expect_success 'add w/no options on existing note morphs into edit' ' > MSG=b2 git notes add && > test_path_is_missing .git/NOTES_EDITMSG && > - test "1" = "$(git ls-tree refs/notes/commits | wc -l)" && > + test "1" = $(git ls-tree refs/notes/commits | wc -l) && > test "b2" = "$(git notes show)" && > git show HEAD^ && > test_must_fail git notes show HEAD^ > @@ -111,7 +111,7 @@ test_expect_success 'add w/no options on existing > note morphs into edit' ' > test_expect_success 'can overwrite existing note with "git notes add -f"' ' > MSG=b1 git notes add -f && > test_path_is_missing .git/NOTES_EDITMSG && > - test "1" = "$(git ls-tree refs/notes/commits | wc -l)" && > + test "1" = $(git ls-tree refs/notes/commits | wc -l) && > test "b1" = "$(git notes show)" && > git show HEAD^ && > test_must_fail git notes show HEAD^ > > > ...Johan > > -- > Johan Herland, <johan@xxxxxxxxxxx> > www.herland.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html