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

Re: [PATCH 1/3] t4117: check for files using `test_path_is_file`

From
Jeff King <peff@peff.net>
Date
Feb 23, 2020, 21:46 UTC
Message-ID
<20200223214650.GB564691@coredump.intra.peff.net>
In-Reply-To
<dde0f85e5e3dd99a61b83df1b6eb572be8a3ff51.1582447606.git.martin.agren@gmail.com>
On Sun, Feb 23, 2020 at 09:48:34AM +0100, Martin Ågren wrote:
Show 9 quoted lines
> We `cat` files, but don't inspect or grab the contents in any way. These
> `cat` calls look like remnants from a debug session, so it's tempting to
> get rid of them. But they do actually verify that the files exist, which
> might not necessarily be the case for some failure modes of `git apply
> --reject`. Let's not lose that.
> 
> Convert the `cat` calls to use `test_path_is_file` instead. This is of
> course still a minor change since we no longer verify that the files can
> be opened for reading, but that is not something we usually worry about.

This reasoning makes sense to me. Thanks for being careful in thinking about possible ramifications of removing these otherwise useless-looking cat statements.

This is and the other two patches all look good to me.
-Peff
Previous: Junio C Hamano
Message 8 of 8 in “t4117: check for files using `test_path_is_file`”
  1. 1/3 t4117: check for files using `test_path_is_file`Martin Ågren, Feb 23, 2020
  2. 2/3 t9810: drop debug `cat` callMartin Ågren, Feb 23, 2020
  3. Junio C HamanoFeb 24, 2020
  4. 3/3 t: drop debug `cat` callsMartin Ågren, Feb 23, 2020
  5. Junio C HamanoFeb 24, 2020
  6. Martin ÅgrenFeb 24, 2020
  7. Junio C HamanoFeb 24, 2020
  8. Jeff KingFeb 23, 2020

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.