Jeff King <peff@xxxxxxxx> wrote: > On Tue, Jun 18, 2024 at 09:30:41PM +0000, Eric Wong wrote: > > > +script=' <snip> > > +expect="$hello_oid blob $hello_size" > > + > > +test_expect_success PERL '--batch-check is unbuffered by default' ' > > + perl -e "$script" -- --batch-check $hello_oid "$expect" > > +' > > We often use "perl -e" for one-liners, etc, but this is pretty big. > Maybe: > > cat >foo.pl <<-\EOF > ... > EOF > perl foo.pl -- ... > > would be more readable? To be clear I don't think there's anything > incorrect about your usage, but it would match the style of our suite a > bit better. *shrug* It doesn't save the nested quoting/expansion confusion; but it's Junio's call. I don't think a v3 is worth the effort. > Likewise, it would be usual in our suite for the helper to do the > minimum that needs to be in perl, and use our normal functions for > things like comparing output (rather than taking its own "expect" > argument). <snip> > +test_expect_success PERL '--batch-check is unbuffered by default' ' > + echo "$hello_oid" | > + perl run-and-wait.pl git cat-file --batch-check >out && > + echo "$hello_oid blob $hello_size" >expect && > + test_cmp expect out I prefer to avoid process spawning overhead from test_cmp; but that's a small drop in a big bucket. > I went for brevity above. Notably missing are: > > - the use of strict/warnings. I think we've shied away from these in > the test suite because we want to run on any version of perl. In my > experience most strict/warnings output is actually telling you about > obvious garbage, but not always. IIRC perl got more strict about > "()" around lists in some contexts a few years back, and code which > used to be OK started generating warnings. OTOH, those warnings were > probably a sign of problems-to-come, anyway. Without "FATAL", > though, I think "use warnings" is not doing much good (nobody is > ever going to see its output if the test isn't failing). It may make problems easier to find if there are failures, so I think the potential benefits outweight any downsides. > - I dropped the close/waitpid. I guess maybe it is valuable to confirm > that cat-file did not barf, but IMHO the important thing here is > testing that it produced the single line of output we expected. I've found some unexpected bugs through excessive error checking in the past, so much preferred to keep them.
