Am 01.01.2017 um 15:23 schrieb Luke Diamand:
On 31 December 2016 at 11:44, Pranit Bauva <pranit.ba...@gmail.com> wrote:
diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh
index 0fe231280..2384535a7 100755
--- a/t/t9813-git-p4-preserve-users.sh
+++ b/t/t9813-git-p4-preserve-users.sh
@@ -126,13 +126,13 @@ test_expect_success 'not preserving user with mixed 
authorship' '
                grep "git author char...@example.com does not match" &&

                make_change_by_user usernamefile3 alice al...@example.com &&
-               git p4 commit |\
-               test_must_fail grep "git author.*does not match" &&
+               ! git p4 commit |\
+               grep "git author.*does not match" &&

Would it be clearer to use this?

    git p4 commit |\
    grep -q -v "git author.*does not match" &&

With your original change, I think that if "git p4 commit" fails, then
that expression will be treated as a pass.

No. The exit code of the upstream in a pipe is ignored. For this reason, having a git invocation as the upstream of a pipe *anywhere* in the test suite is frowned upon. Hence, a better rewrite would be

        git p4 commit >actual &&
        ! grep "git author.*does not match" actual &&

which makes me wonder: Is the message that we do expect not to occur actually printed on stdout? It sounds much more like an error message, i.e., text that is printed on stderr. Wouldn't we need this?

        git p4 commit >actual 2>&1 &&
        ! grep "git author.*does not match" actual &&

-- Hannes

Reply via email to