git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 0/2] microproject: avoid using pipes in test

From
Taylor Blau <me@ttaylorr.com>
Date
Feb 22, 2022, 19:39 UTC
Message-ID
<YhU76TpyP6p56/oV@nand.local>
In-Reply-To
<220222.86pmnf6ket.gmgdl@evledraar.gmail.com>
On Tue, Feb 22, 2022 at 03:24:31PM +0100, Ævar Arnfjörð Bjarmason wrote:
Show 21 quoted lines
>
> On Tue, Feb 22 2022, Shubham Mishra wrote:
>
> > pipes doesn't care about error codes and ignore them thus we should not use them in tests.
>
> Aside from what Derrick Stolee mentioned in his feedback, all of which I
> agree with.
>
> I think these changes are good, but it's not the case that we try to
> avoid using pipes at all in our tests.
>
> It's often a hassle, and just not worth it, e.g.:
>
>     oid=$(echo foo | git hash-object --stdin -w) &&
>
> Sure, we can make that:
>
>     echo foo >in &&
>     oid=$(git hash-object --stdin -w <in) &&
>
> But in the general case it's not worth worrying about.

Agreed, and I would add that we don't necessarily need to worry about non-Git commands on the left-hand side of a pipe. So something like:

    find ... | sort >actual

isn't a problem for us, because our test suite assumes that something like find will not fail. So leaving instances of those alone is OK, but...

Show 15 quoted lines
> What we *do* try to avoid, and what's actually important is to never
> invoke "git" or other programs we invoke on the LHS of a pipe, or to
> otherwise do so in a way that hides potential errors.
>
> That's not isolated to just pipes, but e.g. calling it within helper
> functions that don't &&-chain, but pipes are probably the most common
> offender.
>
> The reason we do that is because in hacking git we may make it error,
> segfault etc. If it's on the LHS of a pipe that failure becomes
> indistinguishable from success.
>
> And if the test is really checking e.g. "when I run git like this, it
> produces no output" printing nothing with an exit of 0 will become the
> same as a segafault for the purposes of test.
...yes, we do care about Git failures. So something like:
    git ls-files | grep "want"

would be no-good, since any failures running 'git ls-files' would be quashed by the pipe.

Thanks, Taylor

Previous: Shubham MishraNext: Shubham Mishra
Message 12 of 31 in “microproject: avoid using pipes in test”
  1. 0/2 microproject: avoid using pipes in testShubham Mishra, Feb 22, 2022
  2. 1/2 t0001: remove pipesShubham Mishra, Feb 22, 2022
  3. Derrick StoleeFeb 22, 2022
  4. Shubham MishraFeb 22, 2022
  5. Christian CouderFeb 22, 2022
  6. Junio C HamanoFeb 23, 2022
  7. Christian CouderFeb 24, 2022
  8. 2/2 t0003: remove pipesShubham Mishra, Feb 22, 2022
  9. Derrick StoleeFeb 22, 2022
  10. Ævar Arnfjörð BjarmasonFeb 22, 2022
  11. Shubham MishraFeb 22, 2022
  12. Taylor BlauFeb 22, 2022
  13. 0/2 avoid pipes with Git on LHSShubham Mishra, Feb 23, 2022
  14. 1/2 t0001: avoid pipes with Git on LHSShubham Mishra, Feb 23, 2022
  15. Taylor BlauFeb 23, 2022
  16. Shubham MishraFeb 23, 2022
  17. Taylor BlauFeb 23, 2022
  18. Junio C HamanoFeb 23, 2022
  19. Shubham MishraFeb 24, 2022
  20. 2/2 t0003: avoid pipes with Git on LHSShubham Mishra, Feb 23, 2022
  21. Taylor BlauFeb 23, 2022
  22. Shubham MishraFeb 23, 2022
  23. Shubham MishraFeb 23, 2022
  24. Shaoxuan YuanFeb 23, 2022
  25. Taylor BlauFeb 23, 2022
  26. Shubham MishraFeb 23, 2022
  27. Christian CouderFeb 24, 2022
  28. Shubham MishraFeb 24, 2022
  29. Christian CouderFeb 24, 2022
  30. Shubham MishraFeb 24, 2022
  31. Christian CouderFeb 24, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.