On Fri, May 20, 2022 at 01:23:22PM +0800, Zorro Lang wrote: > On Thu, May 19, 2022 at 07:02:44PM -0700, Darrick J. Wong wrote: > > On Fri, May 20, 2022 at 11:58:30AM +1000, Dave Chinner wrote: > > > On Thu, May 19, 2022 at 04:13:56PM -0700, Darrick J. Wong wrote: > > > > On Tue, May 17, 2022 at 05:01:04PM +1000, Dave Chinner wrote: > > > > > From: Dave Chinner <dchinner@xxxxxxxxxx> > > > > > > > > > > Because how it actually gets used by the fstests infrastructure > > > > > has been undocumented and that has impact on how it should be set > > > > > up. > > > > > > > > > > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx> > > > > > --- > > > > > README | 41 ++++++++++++++++++++++++++++++++--------- > > > > > 1 file changed, 32 insertions(+), 9 deletions(-) > > > > > > > > > > diff --git a/README b/README > > > > > index 7da66cb6..eacf1acd 100644 > > > > > --- a/README > > > > > +++ b/README > > > > > @@ -368,19 +368,42 @@ Test script environment: > > > > > > > > > > 6. Test group membership: Each test can be associated with any number > > > > > of groups for convenient selection of subsets of tests. Group names > > > > > - can be any sequence of non-whitespace characters. Test authors > > > > > - associate a test with groups by passing the names of those groups as > > > > > - arguments to the _begin_fstest function. For example, the code: > > > > > + can be any sequence of non-whitespace characters, though human-readable > > > > > + names that match the set [A-Za-z0-9\-] are highly prefered. > > > > > > > > > > - _begin_fstest auto quick subvol snapshot > > > > > + Test authors associate a test with groups by passing the names of those > > > > > + groups as arguments to the _begin_fstest function. While _begin_fstests > > > > > + is a shell function that must be called at the start of a test to > > > > > + initialise the test environment correctly, the the build infrastructure > > > > > + also scans the test files for _begin_fstests invocations. It does this > > > > > + to compile the group lists that are used to determine which tests to run > > > > > + when `check` is executed. In other words, test files files must call > > > > > + _begin_fstest with their intended groups or they will not be run. > > > > > + > > > > > + However, because the build infrastructure also uses _begin_fstests as > > > > > + a defined keyword, addition restrictions are placed on how it must be > > > > > + formatted: > > > > > + > > > > > + (a) It must be a single line with no multi-line continuations. > > > > > + > > > > > + (b) group names should be separated by spaces and not other whitespace > > > > > + > > > > > + (c) A '#' placed anywhere in the list, even in the middle of a group > > > > > + name, will cause everything from the # to the end of the line to be > > > > > + ignored. > > > > > > > > I don't see where this is implemented in mkgroupfile? > > > > > > It doesn't need to be. It just aggregates the entire group line, > > > comments and all. Comments *must* be stripped by the thing that reads > > > the group file - mkgroupfile adds comments to every group file it > > > builds. > > > > > > > Was that in the > > > > part of the patchset that got eaten by vger? Or is this patch a > > > > proposal for how we want to define _begin_fstest usage and will be > > > > followed by changes to mkgroupfile to make it do what we now say it > > > > does? > > > > > > It documents the behaviour the mkgroupfile parser currently expects. > > > > Ok. > > > > > > Also, under the old behavior, a '#' not preceded by whitespace or > > > > otherwise escaped on the command line is considered to be part of an > > > > argument: > > > > > > > > $ echo moo#cow > > > > moo#cow > > > > > > Yeah, but we don't need to support that sort of weird thing. The > > > original "Group names can be any sequence of non-whitespace > > > characters" requirement is just a can of worms. > > > > > > > > > > > Not that we /had/ any groups like that. > > > > > > > > Also, I think we ought to add: > > > > > > > > (d) Group names may not contain whitespace or punctuation. > > > > > > > > (e) Quotation marks are considered a part of the group name. > > > > > > The specification after I modified it reads: > > > > > > .... Group names > > > can be any sequence of non-whitespace characters, though > > > human-readable names that match the set [A-Za-z0-9\-] are highly > > > prefered. > > > > > > I'm happy to change that to something like: > > > > > > Group names are to be humand readable names from the > > > character set defined by [:isalnum:\-_]. > > As you haven't sent the next version, I might be a little picky :) > I'm not a regex expert, just tried to run 'grep -E', I think [[:alnum:]_-] might > be better. The 'isalnum' is a function name, the '-' better to be at the end, > or it might be treated as a hyphen. For example: Sure, I wasn't trying to write a fully working regex - that takes thought and this was just a brain dump to indicate roughly what I meant. It looks like you understood it well enough. :) I do intend to change the code to tested and correct regex, then copy it across to the documentation.... FWIW, if you want to improve your regex game, you can get plenty of practice here: https://regexcrossword.com/ Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
