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

Re: [PATCH 1/5] t1300: write test expectations in the test's body

From
Patrick Steinhardt <ps@pks.im>
Date
Sep 15, 2025, 11:41 UTC
Message-ID
<aMf7XvXasOiUAaxo@pks.im>
In-Reply-To
<94eb9052-18e1-4565-8f33-42fdf136e2a4@app.fastmail.com>
On Thu, Sep 11, 2025 at 06:50:29PM +0200, Kristoffer Haugsbakk wrote:
Show 17 quoted lines
> On Thu, Sep 11, 2025, at 15:24, Patrick Steinhardt wrote:
> > There are a bunch of tests in t1300 where we write the test expectation
> > handed over to `test_cmp ()` outside of the test body. This does not
> > match our modern test style, and there isn't really a reason why this
> > would need to happen outside of the test bodies.
> >
> > Convert those to instead do so as part of the test itself.
> >
> > Note that there are two exceptions that we don't convert. In both of
> > these cases the expectation is reused across multiple tests, and thus a
> > conversion where we'd move that into the first test that uses the
> > expectation would be invalid. Those are simply left as-is for now.
> 
> This is just a suggestion (which everything is):
> 
>     Note that there are two exceptions that we leave as-is for now since
>     they are reused across tests.
Yup, that reads way better.
Show 23 quoted lines
> > index f856821839..bde9bda234 100755
> > --- a/t/t1300-config.sh
> > +++ b/t/t1300-config.sh
> > @@ -134,38 +134,39 @@ test_expect_success 'clear default config' '
> >  	rm -f .git/config
> >  '
> >
> > -cat > expect << EOF > +test_expect_success 'initial' '
> > +	cat >expect <<EOF &&
> >  [section]
> >  	penguin = little blue
> >  EOF
> > -test_expect_success 'initial' '
> 
> Ok.  Correct.  But I see that you are also overall doing a sort of
> normalization.
> 
> • Remove-tabs if it makes sense
> • No variable expansion if it makes sense
> 
> And here `<<\EOF` would work too.  Or is that worse style?
> 
> I will keep mentioning this throughout the rest.

No, it's not. I already did it sometimes, but not consistently. I'll mention this in the commit message and adapt as possible.

Show 17 quoted lines
> > @@ -214,7 +213,9 @@ test_expect_success 'Prohibited LF in comment' '
> >  	test_must_fail git config ${mode_set} --comment="a${LF}b" section.k v
> >  '
> >
> > -test_expect_success 'non-match result' 'test_cmp expect .git/config'
> > +test_expect_success 'non-match result' '
> > +	test_cmp expect .git/config
> > +'
> 
> Okay.  You normalize the one line to
> 
>     test_expect_success <name> '
>          <body>
>     '
> 
> The same kind of change done in the next patch `t1300: small
> style fixups`.
True, will move into the next patch.
Thanks for going through all of these!
Patrick
Previous: Kristoffer HaugsbakkNext: Patrick Steinhardt
Message 4 of 36 in “builtin/config: bug fixes for "get" subcommand with "--type=color"”
  1. 0/5 builtin/config: bug fixes for "get" subcommand with "--type=color"Patrick Steinhardt, Sep 11, 2025
  2. 1/5 t1300: write test expectations in the test's bodyPatrick Steinhardt, Sep 11, 2025
  3. Kristoffer HaugsbakkSep 11, 2025
  4. Patrick SteinhardtSep 15, 2025
  5. 2/5 t1300: small style fixupsPatrick Steinhardt, Sep 11, 2025
  6. 3/5 builtin/config: do not die in `get_color()`Patrick Steinhardt, Sep 11, 2025
  7. 4/5 builtin/config: special-case retrieving colors without a keyPatrick Steinhardt, Sep 11, 2025
  8. Kristoffer HaugsbakkSep 11, 2025
  9. Patrick SteinhardtSep 15, 2025
  10. 5/5 builtin/config: do not spawn pager when printing color codesPatrick Steinhardt, Sep 11, 2025
  11. Kristoffer HaugsbakkSep 11, 2025
  12. 0/5 builtin/config: bug fixes for "get" subcommand with "--type=color"Patrick Steinhardt, Sep 15, 2025
  13. 1/5 t1300: write test expectations in the test's bodyPatrick Steinhardt, Sep 15, 2025
  14. 2/5 t1300: small style fixupsPatrick Steinhardt, Sep 15, 2025
  15. 3/5 builtin/config: do not die in `get_color()`Patrick Steinhardt, Sep 15, 2025
  16. 4/5 builtin/config: special-case retrieving colors without a keyPatrick Steinhardt, Sep 15, 2025
  17. Junio C HamanoSep 15, 2025
  18. 5/5 builtin/config: do not spawn pager when printing color codesPatrick Steinhardt, Sep 15, 2025
  19. Junio C HamanoSep 15, 2025
  20. Kristoffer HaugsbakkSep 16, 2025
  21. Patrick SteinhardtSep 18, 2025
  22. 0/5 builtin/config: bug fixes for "get" subcommand with "--type=color"Patrick Steinhardt, Sep 18, 2025
  23. 1/5 t1300: write test expectations in the test's bodyPatrick Steinhardt, Sep 18, 2025
  24. 2/5 t1300: small style fixupsPatrick Steinhardt, Sep 18, 2025
  25. 3/5 builtin/config: do not die in `get_color()`Patrick Steinhardt, Sep 18, 2025
  26. 4/5 builtin/config: special-case retrieving colors without a keyPatrick Steinhardt, Sep 18, 2025
  27. Junio C HamanoSep 18, 2025
  28. Patrick SteinhardtSep 22, 2025
  29. Junio C HamanoSep 22, 2025
  30. 5/5 builtin/config: do not spawn pager when printing color codesPatrick Steinhardt, Sep 18, 2025
  31. 0/5 builtin/config: bug fixes for "get" subcommand with "--type=color"Patrick Steinhardt, Sep 22, 2025
  32. 1/5 t1300: write test expectations in the test's bodyPatrick Steinhardt, Sep 22, 2025
  33. 2/5 t1300: small style fixupsPatrick Steinhardt, Sep 22, 2025
  34. 3/5 builtin/config: do not die in `get_color()`Patrick Steinhardt, Sep 22, 2025
  35. 4/5 builtin/config: special-case retrieving colors without a keyPatrick Steinhardt, Sep 22, 2025
  36. 5/5 builtin/config: do not spawn pager when printing color codesPatrick Steinhardt, Sep 22, 2025

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.