On Sun, Mar 10, 2019 at 5:51 PM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > > On Sun, Mar 10, 2019 at 4:11 AM Jonathan Chang <ttjtftx@xxxxxxxxx> wrote: > > Signed-off-by: Jonathan Chang <ttjtftx@xxxxxxxxx> > > > > diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh > > @@ -1136,8 +1136,8 @@ test_expect_success 'git commit-tree omits duplicated parent in a commit' ' > > - numparent=$(sed -n -e "s/^parent //p" -e "/^author /q" actual | wc -l) && > > - test $numparent = 1 > > + sed -n -e "s/^parent //p" -e "/^author /q" actual | wc -l >numparent && > > + test_line_count = 1 numparent > > This transformation makes no sense. The output of 'sed' is fed to 'wc > -l' whose output is redirected to file "numparent", which means that > the output file will end up containing a single line no matter how > many "parent" lines are matched in the input. Since test_line_count() > checks the number of lines in the named file, the test will succeed > unconditionally (which makes for a pretty poor test). You are right. I will make sure I have thoroughly reviewed my patch before submitting next time. > Also, the filename "numparent" doesn't do a good job of representing > what the file is expected to contain. A better name might be > "parents". So, a more correct transformation would be: > > sed -n -e "s/^parent //p" -e "/^author /q" actual >parents && > test_line_count = 1 parents > > > @@ -1146,8 +1146,8 @@ test_expect_success 'update-index D/F conflict' ' > > - numpath0=$(wc -l <actual) && > > - test $numpath0 = 1 > > + wc -l <actual >numpath0 && > > + test_line_count = 1 numpath0 > > Same comment about bogus transformation. The entire sequence should > collapse to a single line: > > test_line_count = 1 actual Thanks for the help.
