On Mon, Mar 11, 2019 at 6:14 AM Phillip Wood <phillip.wood123@xxxxxxxxx> wrote: > On 11/03/2019 05:49, Eric Sunshine wrote: > > On Sun, Mar 10, 2019 at 11:42 PM Denton Liu <liu.denton@xxxxxxxxx> wrote: > >> + git cat-file -p HEAD |sed -e "1,/^\$/d" >actual && > > > > An earlier patch in this series "fixed" a test with a Git command > > upstream of a pipe. Yet, this test adds such an instance. > > Is it worth worrying about? We're not trying to test cat-file here and > if it does fail the test will fail as actual will be empty. git-cat-file could crash _after_ producing the expected output, in which case we wouldn't learn about the crash at all since the pipe would swallow it. While it's true that we're not specifically testing git-cat-file here, with all the recent effort toward moving away from having Git command upstream of a pipe, I don't think it makes sense to add new cases. And it doesn't hurt to play it safe here: we might actually catch a crash in git-cat-file which isn't caught by other scripts which are genuinely testing that command.
