| Message ID | 1481204070-10881-1-git-send-email-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Dec 08, 2016 at 03:34:30PM +0200, Amir Goldstein wrote: > The -x flag is used to exclude tests that belong to > certain groups from the test args list. > > When the test args list is expressed as a match pattern, > -x fails to exclude the tests that match the pattern > and belong to excluded groups. > > For example: > $ ./check -n -x xfs/??? | wc -l You mean "./check -n -x fuzzers,dangerous_fuzzers | wc -l" here? > 341 > $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l > 341 > > After the fix: > $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l > 315 > > This bug seems to date back to this git repo epoc. > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > check | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/check b/check > index 8f2a1bb..9732460 100755 > --- a/check > +++ b/check > @@ -158,11 +158,14 @@ _timestamp() > _prepare_test_list() > { > unset list > + touch $tmp.list > # Tests specified on the command line > if [ -s $tmp.arglist ]; then > - cat $tmp.arglist > $tmp.list > - else > - touch $tmp.list > + # flatten multi tests line (tests/$fs/???) to 1 test per line > + list=$(cat $tmp.arglist) > + for t in $list; do > + echo "$t" >> $tmp.list > + done Perhaps a sed is more efficient? e.g. - cat $tmp.arglist > $tmp.list + sed 's/ \+/\n/g' $tmp.arglist > $tmp.list Thanks, Eryu -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 9, 2016 at 6:16 AM, Eryu Guan <eguan@redhat.com> wrote: > On Thu, Dec 08, 2016 at 03:34:30PM +0200, Amir Goldstein wrote: >> The -x flag is used to exclude tests that belong to >> certain groups from the test args list. >> >> When the test args list is expressed as a match pattern, >> -x fails to exclude the tests that match the pattern >> and belong to excluded groups. >> >> For example: >> $ ./check -n -x xfs/??? | wc -l > > You mean "./check -n -x fuzzers,dangerous_fuzzers | wc -l" here? > No. I just wanted to present the total number of tests that match the pattern to show in the next line that -x does not exclude any tests. >> 341 >> $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l >> 341 >> >> After the fix: >> $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l >> 315 >> >> This bug seems to date back to this git repo epoc. >> >> Signed-off-by: Amir Goldstein <amir73il@gmail.com> >> --- >> check | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/check b/check >> index 8f2a1bb..9732460 100755 >> --- a/check >> +++ b/check >> @@ -158,11 +158,14 @@ _timestamp() >> _prepare_test_list() >> { >> unset list >> + touch $tmp.list >> # Tests specified on the command line >> if [ -s $tmp.arglist ]; then >> - cat $tmp.arglist > $tmp.list >> - else >> - touch $tmp.list >> + # flatten multi tests line (tests/$fs/???) to 1 test per line >> + list=$(cat $tmp.arglist) >> + for t in $list; do >> + echo "$t" >> $tmp.list >> + done > > Perhaps a sed is more efficient? e.g. > > - cat $tmp.arglist > $tmp.list > + sed 's/ \+/\n/g' $tmp.arglist > $tmp.list > I have considered that and decided that efficiency is not an issue here and better have the robustness of the shell parser without having to worry about all the possible whitespace cases that I may be missing. Besides, this is exactly the same as the population of $tmp.arglist when the args list is expanded by the shell, which is BTW a workaround for this issue, e.g.: $ ln -s tests/xfs xfs $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l 315 Check gets the explicit list of tests in this case and not the match pattern. Amir. -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 9, 2016 at 6:57 AM, Amir Goldstein <amir73il@gmail.com> wrote: > On Fri, Dec 9, 2016 at 6:16 AM, Eryu Guan <eguan@redhat.com> wrote: >> On Thu, Dec 08, 2016 at 03:34:30PM +0200, Amir Goldstein wrote: >>> The -x flag is used to exclude tests that belong to >>> certain groups from the test args list. >>> >>> When the test args list is expressed as a match pattern, >>> -x fails to exclude the tests that match the pattern >>> and belong to excluded groups. >>> >>> For example: >>> $ ./check -n -x xfs/??? | wc -l >> >> You mean "./check -n -x fuzzers,dangerous_fuzzers | wc -l" here? >> > > No. I just wanted to present the total number of tests that match the > pattern to show in the next line that -x does not exclude any tests. > And yes, I have typo. Should be: $ ./check -n xfs/??? | wc -l >>> 341 >>> $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l >>> 341 >>> >>> After the fix: >>> $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l >>> 315 >>> >>> This bug seems to date back to this git repo epoc. >>> >>> Signed-off-by: Amir Goldstein <amir73il@gmail.com> >>> --- >>> check | 9 ++++++--- >>> 1 file changed, 6 insertions(+), 3 deletions(-) >>> >>> diff --git a/check b/check >>> index 8f2a1bb..9732460 100755 >>> --- a/check >>> +++ b/check >>> @@ -158,11 +158,14 @@ _timestamp() >>> _prepare_test_list() >>> { >>> unset list >>> + touch $tmp.list >>> # Tests specified on the command line >>> if [ -s $tmp.arglist ]; then >>> - cat $tmp.arglist > $tmp.list >>> - else >>> - touch $tmp.list >>> + # flatten multi tests line (tests/$fs/???) to 1 test per line >>> + list=$(cat $tmp.arglist) >>> + for t in $list; do >>> + echo "$t" >> $tmp.list >>> + done >> >> Perhaps a sed is more efficient? e.g. >> >> - cat $tmp.arglist > $tmp.list >> + sed 's/ \+/\n/g' $tmp.arglist > $tmp.list >> > > I have considered that and decided that efficiency is not an issue here > and better have the robustness of the shell parser without having to worry > about all the possible whitespace cases that I may be missing. But maybe that just because I am not confident enough about my regexp skills. If folks feel confident about the sed variant, I have no objection. > Besides, this is exactly the same as the population of $tmp.arglist when > the args list is expanded by the shell, which is BTW a workaround for > this issue, e.g.: > > $ ln -s tests/xfs xfs > $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l > 315 > > Check gets the explicit list of tests in this case and not the match pattern. > > Amir. -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 08, 2016 at 03:34:30PM +0200, Amir Goldstein wrote: > The -x flag is used to exclude tests that belong to > certain groups from the test args list. > > When the test args list is expressed as a match pattern, > -x fails to exclude the tests that match the pattern > and belong to excluded groups. > > For example: > $ ./check -n -x xfs/??? | wc -l > 341 > $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l > 341 > > After the fix: > $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l > 315 > > This bug seems to date back to this git repo epoc. > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > check | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/check b/check > index 8f2a1bb..9732460 100755 > --- a/check > +++ b/check > @@ -158,11 +158,14 @@ _timestamp() > _prepare_test_list() > { > unset list > + touch $tmp.list > # Tests specified on the command line > if [ -s $tmp.arglist ]; then > - cat $tmp.arglist > $tmp.list > - else > - touch $tmp.list > + # flatten multi tests line (tests/$fs/???) to 1 test per line > + list=$(cat $tmp.arglist) > + for t in $list; do > + echo "$t" >> $tmp.list > + done Shouldn't the wildcard be evaluated when the CLI argument is being processed and cheked against the group file contents, not hidden in this code? i.e. in the CLI argument processing section starting here: # Process tests from command line now. if $have_test_arg; then .... Cheers, Dave.
On Fri, Dec 09, 2016 at 07:35:25AM +0200, Amir Goldstein wrote: > On Fri, Dec 9, 2016 at 6:57 AM, Amir Goldstein <amir73il@gmail.com> wrote: > > On Fri, Dec 9, 2016 at 6:16 AM, Eryu Guan <eguan@redhat.com> wrote: > >> On Thu, Dec 08, 2016 at 03:34:30PM +0200, Amir Goldstein wrote: > >>> The -x flag is used to exclude tests that belong to > >>> certain groups from the test args list. > >>> > >>> When the test args list is expressed as a match pattern, > >>> -x fails to exclude the tests that match the pattern > >>> and belong to excluded groups. > >>> > >>> For example: > >>> $ ./check -n -x xfs/??? | wc -l > >> > >> You mean "./check -n -x fuzzers,dangerous_fuzzers | wc -l" here? > >> > > > > No. I just wanted to present the total number of tests that match the > > pattern to show in the next line that -x does not exclude any tests. > > > > And yes, I have typo. Should be: > $ ./check -n xfs/??? | wc -l I meant for this, and I pasted the wrong cmdline too.. sorry. > > >>> 341 > >>> $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l > >>> 341 > >>> > >>> After the fix: > >>> $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l > >>> 315 > >>> > >>> This bug seems to date back to this git repo epoc. > >>> > >>> Signed-off-by: Amir Goldstein <amir73il@gmail.com> > >>> --- > >>> check | 9 ++++++--- > >>> 1 file changed, 6 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/check b/check > >>> index 8f2a1bb..9732460 100755 > >>> --- a/check > >>> +++ b/check > >>> @@ -158,11 +158,14 @@ _timestamp() > >>> _prepare_test_list() > >>> { > >>> unset list > >>> + touch $tmp.list > >>> # Tests specified on the command line > >>> if [ -s $tmp.arglist ]; then > >>> - cat $tmp.arglist > $tmp.list > >>> - else > >>> - touch $tmp.list > >>> + # flatten multi tests line (tests/$fs/???) to 1 test per line > >>> + list=$(cat $tmp.arglist) > >>> + for t in $list; do > >>> + echo "$t" >> $tmp.list > >>> + done > >> > >> Perhaps a sed is more efficient? e.g. > >> > >> - cat $tmp.arglist > $tmp.list > >> + sed 's/ \+/\n/g' $tmp.arglist > $tmp.list > >> > > > > I have considered that and decided that efficiency is not an issue here > > and better have the robustness of the shell parser without having to worry > > about all the possible whitespace cases that I may be missing. > > But maybe that just because I am not confident enough about my regexp > skills. If folks feel confident about the sed variant, I have no objection. $tmp.arglist is populated by echo $SRC_DIR/$test_dir/$test_name >> $tmp.arglist which actually is "echo tests/xfs/???", and it is expended to multiple tests in one line by bash in one shot, seperated by only one space. So I think it's safe & quick to do a sed on $tmp.arglist. > > > Besides, this is exactly the same as the population of $tmp.arglist when > > the args list is expanded by the shell, which is BTW a workaround for > > this issue, e.g.: > > > > $ ln -s tests/xfs xfs > > $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l > > 315 This is different, "xfs/???" is expended first by your interactive shell before passing it to check, so check sees multiple tests and iterates over them. Thanks, Eryu -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 9, 2016 at 7:40 AM, Dave Chinner <david@fromorbit.com> wrote: > On Thu, Dec 08, 2016 at 03:34:30PM +0200, Amir Goldstein wrote: >> The -x flag is used to exclude tests that belong to >> certain groups from the test args list. >> >> When the test args list is expressed as a match pattern, >> -x fails to exclude the tests that match the pattern >> and belong to excluded groups. >> >> For example: >> $ ./check -n -x xfs/??? | wc -l >> 341 >> $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l >> 341 >> >> After the fix: >> $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l >> 315 >> >> This bug seems to date back to this git repo epoc. >> >> Signed-off-by: Amir Goldstein <amir73il@gmail.com> >> --- >> check | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/check b/check >> index 8f2a1bb..9732460 100755 >> --- a/check >> +++ b/check >> @@ -158,11 +158,14 @@ _timestamp() >> _prepare_test_list() >> { >> unset list >> + touch $tmp.list >> # Tests specified on the command line >> if [ -s $tmp.arglist ]; then >> - cat $tmp.arglist > $tmp.list >> - else >> - touch $tmp.list >> + # flatten multi tests line (tests/$fs/???) to 1 test per line >> + list=$(cat $tmp.arglist) >> + for t in $list; do >> + echo "$t" >> $tmp.list >> + done > > Shouldn't the wildcard be evaluated when the CLI argument is being > processed and cheked against the group file contents, not hidden in > this code? i.e. in the CLI argument processing section starting > here: > > # Process tests from command line now. > if $have_test_arg; then > .... > Yes, you are right. The test: if egrep "^$test_name" $group_file >/dev/null ; then # in group file ... OK is lousy if $test_name is ??? I moved the expansion to the right place. -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/check b/check index 8f2a1bb..9732460 100755 --- a/check +++ b/check @@ -158,11 +158,14 @@ _timestamp() _prepare_test_list() { unset list + touch $tmp.list # Tests specified on the command line if [ -s $tmp.arglist ]; then - cat $tmp.arglist > $tmp.list - else - touch $tmp.list + # flatten multi tests line (tests/$fs/???) to 1 test per line + list=$(cat $tmp.arglist) + for t in $list; do + echo "$t" >> $tmp.list + done fi # Specified groups to include
The -x flag is used to exclude tests that belong to certain groups from the test args list. When the test args list is expressed as a match pattern, -x fails to exclude the tests that match the pattern and belong to excluded groups. For example: $ ./check -n -x xfs/??? | wc -l 341 $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l 341 After the fix: $ ./check -n -x fuzzers,dangerous_fuzzers xfs/??? | wc -l 315 This bug seems to date back to this git repo epoc. Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- check | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)