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

Re: [PATCH 2/3] t4216: fix broken '&&'-chain

From
Taylor Blau <me@ttaylorr.com>
Date
Jun 30, 2020, 18:39 UTC
Message-ID
<20200630183928.GB26550@syl.lan>
In-Reply-To
<CAPig+cQv3cHe5ci3mDvNKYXbVQt6Rp5icG-woaDCqfAtzZ6SZw@mail.gmail.com>
On Tue, Jun 30, 2020 at 01:50:22PM -0400, Eric Sunshine wrote:
Show 15 quoted lines
> On Tue, Jun 30, 2020 at 1:17 PM Taylor Blau <me@ttaylorr.com> wrote:
> > In a759bfa9ee (t4216: add end to end tests for git log with Bloom
> > filters, 2020-04-06), a 'rm' invocation was added without a
> > corresponding '&&' chain.
> >
> > This ends up working fine when the file already exists, in which case
> > 'rm' exits cleanly and the rest of the function executes normally. When
> > the file does _not_ exist, however, 'rm' returns an unclean exit code,
> > causing the function to terminate.
>
> This explanation makes no sense. Since this command was not part of
> the &&-chain, its failure would not cause the function to terminate
> prematurely nor would it affect the return value of the function. This
> explanation would make sense, however, if you're talking about the
> behavior _after_ fixing the broken &&-chain.

Fair enough. For what it's worth, this explanation *does* make sense if you 'set -e' beforehand, which I am accustomed to (and had incorrectly assumed that tests in 't' also have 'set -e', when they do not).

I've corrected the patch and shortened it to account for your suggestions. Mind taking a look at the updated version and telling me what you think?

--- >8 ---
Subject: [PATCH] t4216: fix broken '&&'-chain

The 'rm' added in a759bfa9ee (t4216: add end to end tests for git log with Bloom filters, 2020-04-06) should be placed within the function's '&&'-chain.

The file being removed may not exist (for eg., in the case of '--run', in which case it may not be generated beforehand by a skipped test), and so add '-f' to account for the file's optional existence.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 t/t4216-log-bloom.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t4216-log-bloom.sh b/t/t4216-log-bloom.sh
index c855bcd3e7..0b4cc4f8d1 100755
--- a/t/t4216-log-bloom.sh
+++ b/t/t4216-log-bloom.sh
@@ -53,7 +53,7 @@ sane_unset GIT_TRACE2_PERF_BRIEF
 sane_unset GIT_TRACE2_CONFIG_PARAMS

 setup () {
-	rm "$TRASH_DIRECTORY/trace.perf"
+	rm -f "$TRASH_DIRECTORY/trace.perf" &&
 	git -c core.commitGraph=false log --pretty="format:%s" $1 >log_wo_bloom &&
 	GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.perf" git -c core.commitGraph=true log --pretty="format:%s" $1 >log_w_bloom
 }
--
2.27.0.224.g4cfa086e50
Previous: Eric SunshineNext: Jeff King
Message 6 of 16 in “commit-graph: introduce 'core.useBloomFilters'”
  1. 0/3 commit-graph: introduce 'core.useBloomFilters'Taylor Blau, Jun 30, 2020
  2. 1/3 commit-graph: pass a 'struct repository *' in more placesTaylor Blau, Jun 30, 2020
  3. Derrick StoleeJun 30, 2020
  4. 2/3 t4216: fix broken '&&'-chainTaylor Blau, Jun 30, 2020
  5. Eric SunshineJun 30, 2020
  6. Taylor BlauJun 30, 2020
  7. Jeff KingJun 30, 2020
  8. Taylor BlauJun 30, 2020
  9. Jeff KingJun 30, 2020
  10. Eric SunshineJun 30, 2020
  11. Jeff KingJun 30, 2020
  12. 3/3 commit-graph: respect 'core.useBloomFilters'Taylor Blau, Jun 30, 2020
  13. Jeff KingJun 30, 2020
  14. Taylor BlauJun 30, 2020
  15. Jeff KingJun 30, 2020
  16. Taylor BlauAug 3, 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.