From: Junio C Hamano <gitster@xxxxxxxxx> > Christian Couder <chriscool@xxxxxxxxxxxxx> writes: > >> Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx> >> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> >> --- >> t/t6050-replace.sh | 40 ++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 40 insertions(+) >> >> diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh >> index fb07ad2..d80a89e 100755 >> --- a/t/t6050-replace.sh >> +++ b/t/t6050-replace.sh >> @@ -18,6 +18,33 @@ add_and_commit_file() >> git commit --quiet -m "$_file: $_msg" >> } >> >> +commit_buffer_contains_parents() > > SP before (), perhaps? Ok. >> +{ >> + git cat-file commit "$1" >payload && >> + sed -n -e '/^$/q' -e '/^parent /p' <payload >actual && >> + shift && >> + : >expected && >> + for _parent >> + do >> + echo "parent $_parent" >>expected || return 1 >> + done && > > You can redirect the entire loop to simplify the above 5 lines, > which would read better, no? > > for _parent > do > echo "parent $_parent" > done >expect Ok, thanks. >> + test_cmp actual expected > > As "test_cmp" normally runs "diff", it is better to compare expect > with actual, not the other way around; running the test verbosely, > i.e. "t6050-replace.sh -v", shows how the actual output differs from > what is expected when diagnosing breakage and would be more useful > that way. Ok. > If your goal is to test both the object contents with this code > *and* the overall system behaviour with the "$child^$parent" below, > perhaps they should be in the same "commit_has_parents" function, > without forcing the caller to choose between the two or call both, > no? Yeah, will do. Thanks, Christian. -- 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