On Fri, Dec 15, 2023 at 1:42 AM Patrick Steinhardt <ps@xxxxxx> 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 keep some of the post-processing of `chainlint.pl` output
> intact to strip leading line numbers generated by the script. 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.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> diff --git a/t/Makefile b/t/Makefile
> @@ -103,20 +103,12 @@ check-chainlint:
> for i in $(CHAINLINTTESTS); do \
> echo "# chainlint: $$i" && \
> - sed -e '/^[ ]*$$/d' chainlint/$$i.expect; \
> + cat chainlint/$$i.expect; \
> done \
> $(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests | \
> - sed -e 's/^[1-9][0-9]* //;/^[ ]*$$/d' >'$(CHAINLINTTMP_SQ)'/actual && \
> + sed -e 's/^[1-9][0-9]* //' >'$(CHAINLINTTMP_SQ)'/actual && \
> + diff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual
Thanks, this version looks fine. FWIW, you may consider this:
Reviewed-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
Aside: I was rather surprised to see this output from git-am when applying v4:
Applying: tests: adjust whitespace in chainlint expectations
.git/rebase-apply/patch:205: new blank line at EOF.
+
.git/rebase-apply/patch:219: new blank line at EOF.
+
warning: 2 lines add whitespace errors.
But upon investigating the two "test" files in question,
dqstring-line-splice.test and dqstring-no-interpolate.test, I recalled
that I had to play tricks to escape the single-quote context created
by the Makefile when generating t/chainlinttmp/tests in order to allow
chainlint.pl to see a double-quoted string. So, the abovementioned
blank lines are indeed expected output from chainlint.pl given the
tricks played.
