Re: [PATCH v2 1/3] test-lib: allow selecting tests by substring/regex with --run

[Elijah Newren <newren@gmail.com>]

On Wed, Oct 14, 2020 at 11:09 AM Jeff King <peff@peff.net> wrote:
>
> On Wed, Oct 14, 2020 at 10:50:03AM -0700, Elijah Newren wrote:
>
> > > > @@ -819,9 +821,8 @@ match_test_selector_list () {
> > > >                       *)
> > > >                               if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null
> > > >                               then
> > > > -                                     echo "error: $title: invalid non-numeric in test" \
> > > > -                                             "selector: '$orig_selector'" >&2
> > > > -                                     exit 1
> > > > +                                     echo "$title" | grep -q "$selector" && return
> > > > +                                     continue
> > > >                               fi
> > >
> > > I like that you allow regexes. It's unfortunate that the skip-check
> > > costs us a process in every test. It may not be that big a deal since we
> > > only pay it if you use a non-numeric selector. But I wonder if there's
> > > any reason not to use "expr" here, as well.
> >
> > I originally used [[ $title =~ "$selector" ]] in order to avoid the
> > sub-shell...but that was bash-specific.  I briefly looked to see if
> > there was a shell portable way to handle regexes, wasn't having much
> > luck, and decided that this is only likely to arise when people are
> > passing --run and thus only running a single script and they avoid all
> > the subprocesses that would have been invoked inside the test, so it's
> > still a big net win overall.  Does expr handle regexes, portably?  Or
> > are you suggesting dropping the regex handling and limit it to
> > substring matching?  In either case, does using expr save us anything
> > (isn't expr a shell command)?
>
> There's an expr command doing a regex match in the diff context quoted
> above. :)

Oh, indeed.  How'd I miss that?

> I was wrong that it would save us a process, though. I thought it was a
> builtin in modern shells, but it doesn't appear to be in either dash or
> bash. So there's little reason to prefer it over grep here (switching to
> globs could be done with case and would save a process).

I guess it's at least slightly more consistent with the surrounding code.

Oh, and I found a bug in that my code was not handling negated
substrings correctly, e.g.
   ./test-script.sh --run="!rename"
would run all the tests instead of excluding the ones that had
"rename" as a substring.  So I'll send a reroll fixing that.