Re: [PATCH] check: fix parsing expunge file with comments

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Mon, Aug 14, 2023 at 05:54:45PM +0300, Amir Goldstein wrote:
> On Mon, Aug 14, 2023 at 5:50 PM Zorro Lang <zlang@xxxxxxxxxx> wrote:
> >
> > On Sun, Aug 13, 2023 at 05:09:50PM +0300, Amir Goldstein wrote:
> > > commit 60054d51 ("check: fix excluded tests are only expunged in the
> > > first iteration") change to use exclude_tests array instead of file.
> > >
> > > The check if a test is in expunge file was using grep -q $TEST_ID FILE
> > > so it was checking if the test was a non-exact match to one of the
> > > lines, for a common example: "generic/001 # exclude this test" would be
> > > a match to test generic/001.
> > >
> > > The commit regressed this example, because the new code checks for exact
> > > match of [ "generic/001" == "generic/001 " ]. Change the code to use
> > > prefix match to deal with this case and any other suffix correctly.
> > >
> > > While the original code would also match line "exclude test generic/001"
> > > I don't think that non-prefix match is a real use case.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > > ---
> > >  check | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/check b/check
> > > index 549725eb..e8152f59 100755
> > > --- a/check
> > > +++ b/check
> > > @@ -592,7 +592,8 @@ _expunge_test()
> > >       local TEST_ID="$1"
> > >
> > >       for f in "${exclude_tests[@]}"; do
> > > -             if [ "${TEST_ID}" == "$f" ]; then
> > > +             # $f may contain traling spaces and comments
> > > +             if [[ "$f" == ${TEST_ID}* ]]; then
> >
> > We'll have seqnum with 4 numbers in one day, likes generic/1000. At that time,
> > generic/100[0-9] will match generic/100*. That's not what we want.
> >
> 
> Good point.
> 
> > I think we'd better to do exact match, or better to match the end of a word,
> > for example
> >
> >   local id_regex="^${TEST_ID}\b"
> >   if [[ "$f" =~ ${id_regex} ]];then
> >   ...
> >
> > I think this will match "generic/100" and "generic/100 # xxxxx". But won't match
> > "generic/1001".
> >
> 
> Nice. Looks good to me.

I didn't test it, please check if that can help you and won't break old things,
if you'd like to write that in V2 :)

Thanks,
Zorro

> 
> Thanks,
> Amir.
> 




[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux