On Tue, Jul 2, 2024 at 5:19 PM Jeff King <peff@xxxxxxxx> wrote:
> On Mon, Jul 01, 2024 at 08:51:44PM -0400, Jeff King wrote:
> > Obviously we'd want to add to the chainlint tests here. It looks like
> > the current test infrastructure is focused on evaluating snippets, with
> > the test_expect_success part already handled.
>
> So doing this (with the patch I showed earlier):
>
> diff --git a/t/Makefile b/t/Makefile
> @@ -106,18 +106,28 @@ clean: clean-except-prove-cache
> + for i in $$(grep -L "'" $(CHAINLINTTESTS_SRC)); do \
> + echo "test_expect_success '$$i' - <<\\\\EOT" && \
> + sed -e '/^# LINT: /d' $$i && \
> + echo "EOT"; \
> + done >>'$(CHAINLINTTMP_SQ)'/tests && \
Unfortunately, `grep -L` is not POSIX.
> does pass. It's just running all of the tests inside an "EOT" block. But
> we have to omit ones that have single quotes in them, because they are
> making the implicit assumption that they're inside a single-quoted block
> (so they do things like '"$foo"', or '\'', etc, which behave differently
> in a here-doc).
>
> It was a nice check that the output is the same in both cases, but it's
> a bit limiting as a test suite, as there's no room to introduce test
> cases that vary the test_expect_success lines.
Agreed. It feels rather hacky and awfully special-case, as it's only
(additionally) checking that the `test_expect_* title - <<EOT` form
works, but doesn't help at all with testing other parsing-related
behaviors of chainlint.pl (which is something I definitely wanted to
be able to do when implementing the Perl version).
> I'm thinking the path forward may be:
>
> 1. Move the test_expect_success wrapping lines into each
> chainlint/*.test file. It's a little bit of extra boilerplate, but
> it makes them a bit easier to reason about on their own.
Yes. This is exactly what I had in mind for moving forward. It's just
a one-time noise-patch cost but gives us much more flexibility in
terms of testing.
It also makes spot-testing the chainlint self-test files much simpler.
We would be able to do this:
./chainlint.pl chainlint/block.test
rather than much more painful:
{ echo "test_expect_success foo '" && cat chainlint/block.test &&
echo "'"; } >dummy && ./chainlint.pl dummy; rm dummy
or something similar.
> 2. Add a few new tests that use here-docs with a few variations
> ("<<EOT", "<<\EOT", probably a here-doc inside the test here-doc).
>
> Does that sound OK to you?
Absolutely. I'm very much in favor of these changes.
