Re: [PATCH v2] tests: adjust whitespace in chainlint expectations
From: Eric Sunshine <hidden>
Date: 2023-12-14 08:44:55
On Thu, Dec 14, 2023 at 3:30 AM Patrick Steinhardt [off-list ref] wrote:
quoted hunk ↗ jump to hunk
[...] Instead of improving the detection logic, fix our ".expect" files so that we do not need any post-processing anymore. This allows us to drop the `-w` flag when diffing so that we can always use diff(1) now. Signed-off-by: Patrick Steinhardt <redacted> ---diff --git a/t/Makefile b/t/Makefile@@ -108,15 +108,7 @@ check-chainlint: } >'$(CHAINLINTTMP_SQ)'/expect && \ $(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests | \ sed -e 's/^[1-9][0-9]* //;/^[ ]*$$/d' >'$(CHAINLINTTMP_SQ)'/actual && \ - if test -f ../GIT-BUILD-OPTIONS; then \ - . ../GIT-BUILD-OPTIONS; \ - fi && \ - if test -x ../git$$X; then \ - DIFFW="../git$$X --no-pager diff -w --no-index"; \ - else \ - DIFFW="diff -w -u"; \ - fi && \ - $$DIFFW '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual + diff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual
This change feels incomplete. Yes, it "corrects" the whitespace, but check-chainlint still applies sed to remove blank lines and strip the line numbers from the beginning of each line. Based upon the commit message, I had expected that all post-processing of the output would have been eliminated and that the "expect" files would exactly mirror the output of the linter. As mentioned elsewhere[1], I'm not particularly in favor of this approach, though I won't stand in the way of it either. [1]: https://lore.kernel.org/git/CAPig+cSkuRfkR2D3JqYcbaJqj485nfD9Nq6pM=vXWB5DJenWpA@mail.gmail.com/ (local)