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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Dec 15, 2023, 07:17 UTC
Message-ID: <CAPig+cSNZ75WSjxfJFoVPBTkv7yQpxGz1c4ugCyQRLytpNLjig@mail.gmail.com>
In-Reply-To: <fb312f559de7b99244e4c86a995250599cd9be06.1702622508.git.ps@pks.im>

On Fri, Dec 15, 2023 at 1:42 AM Patrick Steinhardt <ps@pks.im> wrote:

Show 25 quoted lines

> [...]
> 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@pks.im>
> ---
> 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@sunshineco.com>

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.