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

Re: [PATCH v3] tests: adjust whitespace in chainlint expectations

From
Patrick Steinhardt <ps@pks.im>
Date
Dec 15, 2023, 06:29 UTC
Message-ID
<ZXvyL2wtoTIt6OVc@tanuki>
In-Reply-To
<CAPig+cQozi+aiTc5Bve4OHrfuSRGUCSkKmhoYtkGTmn64Ps-rw@mail.gmail.com>
On Fri, Dec 15, 2023 at 01:24:20AM -0500, Eric Sunshine wrote:
Show 29 quoted lines
> On Fri, Dec 15, 2023 at 1:04 AM Patrick Steinhardt <ps@pks.im> wrote:
> > [...]
> > Instead of improving the detection logic, fix our ".expect" files so
> > that we do not need any post-processing at all anymore. This allows us
> > to drop the `-w` flag when diffing so that we can always use diff(1)
> > now.
> >
> > Note that we leave the post-processing of `chainlint.pl` output intact.
> > All we do here is to strip leading line numbers that it would otherwise
> > generate.
> 
> Hmm, okay, but... (see below)
> 
> > Having these would cause a rippling effect whenever we add a
> > new test that sorts into the middle of existing tests and would require
> > us to renumerate all subsequent lines, which seems rather pointless.
> 
> Just an aside, not strictly relevant at this time: Ævar has proposed
> that check-chainlint should not be creating conglomerate "test",
> "expect", and "actual" files, but should instead let `make` run
> chainlint.pl separately on each chainlint self-test file, thus
> benefiting from `make`'s innate parallelism rather than baking
> parallelism into chainlint.pl itself. More importantly, `make`'s
> dependency tracking would ensure that a chainlint self-test file only
> gets rechecked if its timestamp changes. That differs from the current
> situation in which _all_ of the chainlint self-test files are checked
> on _every_ `make test` which is wasteful if none of them have changed.
> Anyhow, with his proposed approach, there wouldn't be cascading line
> number changes just because a new self-test file was added.

I was indeed also thinking along this way and would tend to agree. I punted on it as I honestly only really care for fixing the immediate issue that the post-processing causes for me.

Are you fine with deferring this bigger refactoring?
Show 13 quoted lines
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > diff --git a/t/Makefile b/t/Makefile
> > @@ -103,20 +103,12 @@ check-chainlint:
> >         $(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests | \
> >                 sed -e 's/^[1-9][0-9]* //;/^[   ]*$$/d' >'$(CHAINLINTTMP_SQ)'/actual && \
> 
> The commit message claims that this is only stripping the line numbers
> which prefix each emitted line, but the `/^[   ]*$$/d` bit is also
> deleting blank lines from the output of chainlint.pl. Thus, this ought
> to be:
> 
>     sed -e 's/^[1-9][0-9]* //' >'$(CHAINLINTTMP_SQ)'/actual && \
Gah, you're right, I missed the second part. Will fix in another round.
Patrick
Previous: Eric SunshineNext: Eric Sunshine
Message 17 of 21 in “tests: prefer host Git to verify chainlint self-checks”
  1. tests: prefer host Git to verify chainlint self-checksPatrick Steinhardt, Dec 12, 2023
  2. Junio C HamanoDec 12, 2023
  3. Patrick SteinhardtDec 13, 2023
  4. Junio C HamanoDec 13, 2023
  5. Eric SunshineDec 14, 2023
  6. Patrick SteinhardtDec 14, 2023
  7. Eric SunshineDec 14, 2023
  8. Patrick SteinhardtDec 14, 2023
  9. Junio C HamanoDec 14, 2023
  10. Eric SunshineDec 14, 2023
  11. Junio C HamanoDec 14, 2023
  12. Patrick SteinhardtDec 15, 2023
  13. tests: adjust whitespace in chainlint expectationsPatrick Steinhardt, Dec 14, 2023
  14. Eric SunshineDec 14, 2023
  15. tests: adjust whitespace in chainlint expectationsPatrick Steinhardt, Dec 15, 2023
  16. Eric SunshineDec 15, 2023
  17. Patrick SteinhardtDec 15, 2023
  18. Eric SunshineDec 15, 2023
  19. tests: adjust whitespace in chainlint expectationsPatrick Steinhardt, Dec 15, 2023
  20. Eric SunshineDec 15, 2023
  21. Junio C HamanoDec 15, 2023

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.