On Fri, Jan 15, 2021 at 10:14:40AM +0100, Ævar Arnfjörð Bjarmason wrote: > > On Fri, Jan 15 2021, Ævar Arnfjörð Bjarmason wrote: > > > test_expect_success 'cherry picks did not become git merge commits' ' > > bad_cherries=$(git rev-list --parents --all --grep=Cherry | > > - grep " .* " | cut -f1 -d" ") && > > + test_might_fail ok=sigpipe grep grep " .* " | cut -f1 -d" ") && > > [...] > > - grep " .* " | cut -f1 -d" ") && > > + test_might_fail ok=sigpipe grep grep " .* " | cut -f1 -d" ") && > > So, "grep grep" is an obvious typo there. Oops. The test still passes > because it's fragile to begin with, we're just checking that we get no > output, so "grep this string is not here" would also pass. This is a good example of why tests are often better written to check for an expected outcome, rather than lack of an unexpected one. That does often make tests more verbose and brittle, though (in this case, I guess we'd presumably have to validate the whole "rev-list" output. So it may not be practical. I do wonder if this test needs these pipes at all. It looks like we are looking for merge commits. Maybe "rev-list --merges" would be a bit simpler these days? I.e.: diff --git a/t/t9151-svn-mergeinfo.sh b/t/t9151-svn-mergeinfo.sh index 4f6c06ecb2..20dd62c5c4 100755 --- a/t/t9151-svn-mergeinfo.sh +++ b/t/t9151-svn-mergeinfo.sh @@ -17,22 +17,19 @@ test_expect_success 'load svn dump' " " test_expect_success 'all svn merges became git merge commits' ' - unmarked=$(git rev-list --parents --all --grep=Merge | - grep -v " .* " | cut -f1 -d" ") && - [ -z "$unmarked" ] - ' + git rev-list --all --no-merges --grep=Merge >actual && + test_must_be_empty actual +' test_expect_success 'cherry picks did not become git merge commits' ' - bad_cherries=$(git rev-list --parents --all --grep=Cherry | - grep " .* " | cut -f1 -d" ") && - [ -z "$bad_cherries" ] - ' + git rev-list --all --merges --grep=Cherry >actual && + test_must_be_empty actual +' test_expect_success 'svn non-merge merge commits did not become git merge commits' ' - bad_non_merges=$(git rev-list --parents --all --grep=non-merge | - grep " .* " | cut -f1 -d" ") && - [ -z "$bad_non_merges" ] - ' + git rev-list --all --merges --grep=non-merge >actual && + test_must_be_empty actual +' test_expect_success 'commit made to merged branch is reachable from the merge' ' before_commit=$(git rev-list --all --grep="trunk commit before merging trunk to b2") && -Peff
