Re: [PATCH 04/12] fstests: fix group list generation for whacky test names

From: Zorro Lang <zlang@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: fstests@vger.kernel.org
Subject: Re: [PATCH 04/12] fstests: fix group list generation for whacky test names
Date: Fri, 20 May 2022 16:54:45 +0800	[thread overview]
Message-ID: <20220520085445.mil6iwc5mltbmss3@zlang-mailbox> (raw)
In-Reply-To: <20220520083647.s22yb2aearmviffi@zlang-mailbox>

On Fri, May 20, 2022 at 04:36:47PM +0800, Zorro Lang wrote:
> On Tue, May 17, 2022 at 05:01:03PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Darrick noticed that tests/xfs/191-input-validation didn't get
> > generated properly. Fix the regex to handle this.
> > 
> > $ grep -I -R "^_begin_fstest" tests/xfs | \
> >   sed -e 's/^.*\/\([0-9]*\):_begin_fstest/\1/' |grep 191
> > tests/xfs/191-input-validation:_begin_fstest auto quick mkfs realtime
> > $
> > $ grep -I -R "^_begin_fstest" tests/xfs | \
> >   sed -e 's/^.*\/\([0-9]*\).*:_begin_fstest/\1/ ' |grep 191
> > 191 auto quick mkfs realtime
> > $
> > 
> > Use the regexes for matching test names defined in common/test_names
> > rather than trying to open code it.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > ---
> 
> Hi Dave,
> 
> After testing, looks like this patch brings in a regression issue, it causes
> case number aren't sorted in group.list, then the xfstests/new program can't
> get a right new case number which is unused. For example:

Oh, this issue isn't from this patch, it's from 441606d2 ("fstests: faster group
file creation"). Hmm... I saw your used "sort -u" in that patch, I'm going to
look into what's wrong with that.

Thanks,
Zorro

> 
> # make
> # ./new generic
> Next test id is 019
> Append a name to the ID? Test name will be 019-$name. y,[n]: ^C
> # ls tests/generic/019
> tests/generic/019
> 
> Then comparing the tests/generic/group.list with old generic/group.list, found
> lots of difference, due to the number in new group.list isn't sorted from
> small to big (I haven't gotten chance to check if there're missing number).
> 
> Thanks,
> Zorro
> 
> >  tools/mkgroupfile | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/mkgroupfile b/tools/mkgroupfile
> > index 24435898..414cb538 100755
> > --- a/tools/mkgroupfile
> > +++ b/tools/mkgroupfile
> > @@ -19,6 +19,8 @@ if [ ! -x ../../check ]; then
> >  	exit 1
> >  fi
> >  
> > +. ../../common/test_names
> > +
> >  cleanup()
> >  {
> >  	rm -f $new_groups.check
> > @@ -60,7 +62,8 @@ ENDL
> >  
> >  	# Aggregate the groups each test belongs to for the group file
> >  	grep -I -R "^_begin_fstest" $test_dir/ | \
> > -		sed -e 's/^.*\/\([0-9]*\):_begin_fstest/\1/' >> $new_groups
> > +		sed -e "s/^.*\/\($VALID_TEST_NAME\):_begin_fstest/\1/" \
> > +		>> $new_groups
> >  
> >  	# Create the list of unique groups for existence checking
> >  	grep -I -R "^_begin_fstest" $test_dir/ | \
> > -- 
> > 2.35.1
> > 