gitster@xxxxxxxxx wrote on Tue, 26 Jun 2012 09:26 -0700: > Johannes Sixt <j.sixt@xxxxxxxxxxxxx> writes: > > > Am 6/26/2012 3:18, schrieb Pete Wyckoff: > >> test_expect_success 'exit when p4 fails to produce marshaled output' ' > >> - badp4dir="$TRASH_DIRECTORY/badp4dir" && > >> - mkdir "$badp4dir" && > >> - test_when_finished "rm \"$badp4dir/p4\" && rmdir \"$badp4dir\"" && > >> - cat >"$badp4dir"/p4 <<-EOF && > >> + mkdir badp4dir && > >> + test_when_finished "rm badp4dir/p4 && rmdir badp4dir" && > >> + cat >badp4dir/p4 <<-EOF && > >> #!$SHELL_PATH > >> exit 1 > >> EOF > >> - chmod 755 "$badp4dir"/p4 && > >> - PATH="$badp4dir:$PATH" git p4 clone --dest="$git" //depot >errs 2>&1 ; retval=$? && > >> + chmod 755 badp4dir/p4 && > >> + PATH="$TRASH_DIRECTORY/badp4dir:$PATH" git p4 clone --dest="$git" //depot >errs 2>&1 ; retval=$? && > >> test $retval -eq 1 && > > > > The long line here is severly broken, because the semicolon breaks the && > > chain; retval would be assigned to even if one of the earlier commands > > fails, and that you don't want to treat as success. The least that is > > needed is to put the line in braces. But I suggest to rewrite the two > > lines above as > > > > ( > > PATH="$TRASH_DIRECTORY/badp4dir:$PATH" && > > export PATH && > > test_expect_code 1 git p4 clone --dest="$git" //depot >errs 2>&1 > > ) && > > > >> test_must_fail grep -q Traceback errs > > > > We don't expect that grep fails due to segfault or something. Write this > > line as > > > > ! grep Traceback errs > > > > Also drop the -q; if the test detects a failure, you do want to see the > > grep output in a verbose test run. > > All true. Thanks for carefully reading. Yes, both your reviews are quite appreciated. I've got patches ready for these and will plan to send out an updated series tonight. -- Pete -- 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