Re: [PATCH v2 4/8] test-lib-functions: add and use test_cmp_cmd
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 2, 2022, 03:41 UTC
- Message-ID
- <xmqq5yeupj53.fsf@gitster.g>
- In-Reply-To
- <CAPig+cSgnhzFCUVTQCcTKJK+2qVOpdB2R-Vq1DjqspDJudom4w@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
Show 21 quoted lines
> Another reason I'm "meh" about this function is that it seems too > narrowly focussed, insisting that the "expect" argument is expressed > as a one-liner. (Yes, I know that that is not a hard limitation; a > caller can pass in a multiline string, but still...) Maybe I'd be less > jaded if it accepted "expect" on its stdin. But, even that doesn't > seem to buy much. The vast majority of cases where you've converted: > > test "$dir" = "$(test-tool path-utils real_path $dir2)" && > > to: > > test_cmp_cmd "$dir" test-tool path-utils real_path $dir2 && > > could just have easily become: > > echo "$dir" >expect && > test-tool path-utils real_path $dir2 >actual && > test_cmp expect actual > > which isn't bad at all, even if it is one line longer, and it is > idiomatic in this test suite.
[jc: updated the rewritten example]
And it is crystal clear that "expect" and "actual" are clobbered if written that way.