On Mon, May 16, 2022 at 09:20:26AM -0700, Darrick J. Wong wrote: > On Mon, May 16, 2022 at 06:59:19PM +1000, Dave Chinner wrote: > > From: Dave Chinner <dchinner@xxxxxxxxxx> > > > > 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 > > $ > > > > Long term, we should rename that test to '191' and rip out all that > > unused and unnecessary complexity for matching ascii test names > > because we just don't use it. Numbers for tests are still working > > just fine. > > > > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx> > > --- > > tools/mkgroupfile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/mkgroupfile b/tools/mkgroupfile > > index 24435898..958d4e2f 100755 > > --- a/tools/mkgroupfile > > +++ b/tools/mkgroupfile > > @@ -60,7 +60,7 @@ 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/^.*\/\([0-9]*\).*:_begin_fstest/\1/' >> $new_groups > > Sorry I didn't get a chance to review this patch before it went in, but > this string parsing gets tripped up by things that the old code handled > just fine. Back when we'd run _begin_fstest as a real bash subroutine > to print the group name arguments, a line like this: > > _begin_fstest deprecated # log logprint quota And I just don't care about that. Handling these whacky corner cases is the *wrong solution* - we're just creating more technical debt for ourselves going down that path. $ git grep _begin_fstest tests/ |grep "#" tests/xfs/018:_begin_fstest deprecated # log logprint v2log tests/xfs/081:_begin_fstest deprecated # log logprint quota tests/xfs/082:_begin_fstest deprecated # log logprint v2log $ There are the only 3 tests marked deprecated. *Nobody runs them* and if they did, they all fail on a current kernel: .... Running: MOUNT_OPTIONS= ./check -R xunit -b -s xfs xfs/018 xfs/081 xfs/082 .... Failures: xfs/018 xfs/081 xfs/082 Failed 3 of 3 tests We don't need to support this whacky corner case - just remove the three deprecated tests completely. If anyone needs them, they are still in the git history. But the right thing to do is to remove the corner case altogether, not add complexity to handle it needlessly. > Instead, that above output (which I harvested from xfs/081) now becomes: > > 081 deprecated # log logprint quota > > The first grepsed blobule should do more if it's going to > performance-optimize bash: We don't need to "performance-optimize bash" - we've already fixed the problem by addressing the low hanging fruit (35s down to less than 0.4s, and less than 0.1s with make -j8). Beyond where we iare now is really "don't care" territory because nobody is going to notice it going 0.1s faster now. OTOH, we are sure as anything going to notice the complexity of the regexes in a year's time when we have to work out what it does from first principles again.... > grep -I -R "^_begin_fstest" -Z $test_dir/ | \ > sed -e 's/#.*$//g' \ This is the only bit that is needed to handle the commented out group case, everythign else is just complexity that comes from over-optimisation. And even handling comments is largely unnecessary because removing deprecated tests is the right way to fix this... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
