On Fri, Mar 24, 2023 at 11:17:11PM +0100, Michael J Gruber wrote:
> 1f2e05f0b7 ("wildmatch: fix exponential behavior", 2023-03-20)
> introduced a new test with a background process. Backgrounding
> necessarily gives a result of 0, so that a seemingly broken && chain is
> not really broken.
>
> Adjust t3070 slightly so that our chain list test recognizes the
> construct for what it is and does not raise a false positive.
Good catch. While I agree that there's no missed exit code here, I'd say
that this is more than just a false positive. If there were any lines
above the "&", like:
foo &&
bar &
pid=$! &&
...etc...
then we'd be losing the exit value of "foo". It's OK here because the
backgrounded command is the first line of the test, but it definitely
violates our guidelines.
Which isn't to say that your patch needs to do anything differently. I
just wondered if it meant we should be improving the chain linter, but I
think it is doing the right thing to alert us here.
> diff --git a/t/t3070-wildmatch.sh b/t/t3070-wildmatch.sh
> index b91a7cb712..4dd42df38c 100755
> --- a/t/t3070-wildmatch.sh
> +++ b/t/t3070-wildmatch.sh
> @@ -432,10 +432,12 @@ match 0 1 0 1 'z' '[Z-y]'
> match 1 1 1 1 'Z' '[Z-y]'
>
> test_expect_success 'matching does not exhibit exponential behavior' '
> - test-tool wildmatch wildmatch \
> - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab \
> - "*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a" &
> - pid=$! &&
> + {
> + test-tool wildmatch wildmatch \
> + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab \
> + "*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a*a" &
> + pid=$!
> + } &&
> sleep 2 &&
> ! kill $!
This looks like the right solution. I do wonder how Phillip managed to
miss it, though, since the test script complains loudly.
-Peff
