All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] check: run auto test group by default
@ 2018-05-04  0:05 Dave Chinner
  2018-05-04  3:06 ` Matthew Wilcox
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Dave Chinner @ 2018-05-04  0:05 UTC (permalink / raw)
  To: fstests; +Cc: willy

From: Dave Chinner <dchinner@redhat.com>

Everyone who starts using fstests runs "check" without parameters,
and then has problems with it running dangerous tests. most people
just want fstests to act as a regression test suite, not a fuzzer or
exercise known crash conditions. Hence make the default behaviour to
be "run the auto group" rather than "run every test".

To enable people to run all tests easily (if they really want to)
add a special group keyword named "all". This wildcard will trigger
selecting all the tests in fstests as per the original "check
without parameters" behaviour.

Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
 check | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/check b/check
index 84a35e6724ab..170004d4b2a2 100755
--- a/check
+++ b/check
@@ -102,6 +102,8 @@ a test file name match pattern (e.g. xfs/*).
 group argument is either a name of a tests group to collect from all
 the test dirs (e.g. quick) or a name of a tests group to collect from
 a specific tests dir in the form of <test dir>/<group name> (e.g. xfs/quick).
+If you want to run all the tests in teh test suite, use "-g all" to specify all
+groups.
 
 exclude_file argument refers to a name of a file inside each test directory.
 for every test dir where this file is found, the listed test names are
@@ -220,22 +222,24 @@ _prepare_test_list()
 	fi
 
 	# Specified groups to include
-	for group in $GROUP_LIST; do
-		list=$(get_group_list $group)
-		if [ -z "$list" ]; then
-			echo "Group \"$group\" is empty or not defined?"
-			exit 1
-		fi
+	# Note that the CLI processing adds a leading space to the first group
+	# parameter, so we have to catch that here checking for "all"
+	if ! $have_test_arg && [ "$GROUP_LIST" == " all" ]; then
+		# no test numbers, do everything
+		get_all_tests
+	else
+		for group in $GROUP_LIST; do
+			list=$(get_group_list $group)
+			if [ -z "$list" ]; then
+				echo "Group \"$group\" is empty or not defined?"
+				exit 1
+			fi
 
-		for t in $list; do
-			grep -s "^$t\$" $tmp.list >/dev/null || \
+			for t in $list; do
+				grep -s "^$t\$" $tmp.list >/dev/null || \
 							echo "$t" >>$tmp.list
+			done
 		done
-	done
-
-	if ! $have_test_arg && [ -z "$GROUP_LIST" ]; then
-		# no test numbers, do everything
-		get_all_tests
 	fi
 
 	# Specified groups to exclude
@@ -364,6 +368,10 @@ if $have_test_arg; then
 
 		shift
 	done
+elif [ -z "$GROUP_LIST" ]; then
+	# default group list is the auto group. If any other group or test is
+	# specified, we use that instead.
+	GROUP_LIST="auto"
 fi
 
 # we need common/rc
-- 
2.17.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] check: run auto test group by default
  2018-05-04  0:05 [PATCH] check: run auto test group by default Dave Chinner
@ 2018-05-04  3:06 ` Matthew Wilcox
  2018-05-04  3:13 ` Eric Sandeen
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Matthew Wilcox @ 2018-05-04  3:06 UTC (permalink / raw)
  To: Dave Chinner; +Cc: fstests

On Fri, May 04, 2018 at 10:05:09AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Everyone who starts using fstests runs "check" without parameters,
> and then has problems with it running dangerous tests. most people
> just want fstests to act as a regression test suite, not a fuzzer or
> exercise known crash conditions. Hence make the default behaviour to
> be "run the auto group" rather than "run every test".

Thanks, Dave.  This makes a lot of sense to me.  I don't feel like I
understand the shell script well enough to offer a meaningful review
though.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] check: run auto test group by default
  2018-05-04  0:05 [PATCH] check: run auto test group by default Dave Chinner
  2018-05-04  3:06 ` Matthew Wilcox
@ 2018-05-04  3:13 ` Eric Sandeen
  2018-05-04  3:50 ` Darrick J. Wong
  2018-05-04  5:55 ` Amir Goldstein
  3 siblings, 0 replies; 7+ messages in thread
From: Eric Sandeen @ 2018-05-04  3:13 UTC (permalink / raw)
  To: Dave Chinner, fstests; +Cc: willy

On 5/3/18 7:05 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Everyone who starts using fstests runs "check" without parameters,
> and then has problems with it running dangerous tests. most people
> just want fstests to act as a regression test suite, not a fuzzer or
> exercise known crash conditions. Hence make the default behaviour to
> be "run the auto group" rather than "run every test".
> 
> To enable people to run all tests easily (if they really want to)
> add a special group keyword named "all". This wildcard will trigger
> selecting all the tests in fstests as per the original "check
> without parameters" behaviour.
> 
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>

I haven't tested but I assume you have ;) and this looks OK to me.

I agree that this makes things much more friendly to the new user,
thanks.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  check | 34 +++++++++++++++++++++-------------
>  1 file changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/check b/check
> index 84a35e6724ab..170004d4b2a2 100755
> --- a/check
> +++ b/check
> @@ -102,6 +102,8 @@ a test file name match pattern (e.g. xfs/*).
>  group argument is either a name of a tests group to collect from all
>  the test dirs (e.g. quick) or a name of a tests group to collect from
>  a specific tests dir in the form of <test dir>/<group name> (e.g. xfs/quick).
> +If you want to run all the tests in teh test suite, use "-g all" to specify all
> +groups.
>  
>  exclude_file argument refers to a name of a file inside each test directory.
>  for every test dir where this file is found, the listed test names are
> @@ -220,22 +222,24 @@ _prepare_test_list()
>  	fi
>  
>  	# Specified groups to include
> -	for group in $GROUP_LIST; do
> -		list=$(get_group_list $group)
> -		if [ -z "$list" ]; then
> -			echo "Group \"$group\" is empty or not defined?"
> -			exit 1
> -		fi
> +	# Note that the CLI processing adds a leading space to the first group
> +	# parameter, so we have to catch that here checking for "all"
> +	if ! $have_test_arg && [ "$GROUP_LIST" == " all" ]; then
> +		# no test numbers, do everything
> +		get_all_tests
> +	else
> +		for group in $GROUP_LIST; do
> +			list=$(get_group_list $group)
> +			if [ -z "$list" ]; then
> +				echo "Group \"$group\" is empty or not defined?"
> +				exit 1
> +			fi
>  
> -		for t in $list; do
> -			grep -s "^$t\$" $tmp.list >/dev/null || \
> +			for t in $list; do
> +				grep -s "^$t\$" $tmp.list >/dev/null || \
>  							echo "$t" >>$tmp.list
> +			done
>  		done
> -	done
> -
> -	if ! $have_test_arg && [ -z "$GROUP_LIST" ]; then
> -		# no test numbers, do everything
> -		get_all_tests
>  	fi
>  
>  	# Specified groups to exclude
> @@ -364,6 +368,10 @@ if $have_test_arg; then
>  
>  		shift
>  	done
> +elif [ -z "$GROUP_LIST" ]; then
> +	# default group list is the auto group. If any other group or test is
> +	# specified, we use that instead.
> +	GROUP_LIST="auto"
>  fi
>  
>  # we need common/rc
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] check: run auto test group by default
  2018-05-04  0:05 [PATCH] check: run auto test group by default Dave Chinner
  2018-05-04  3:06 ` Matthew Wilcox
  2018-05-04  3:13 ` Eric Sandeen
@ 2018-05-04  3:50 ` Darrick J. Wong
  2018-05-04 14:58   ` Eryu Guan
  2018-05-04  5:55 ` Amir Goldstein
  3 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2018-05-04  3:50 UTC (permalink / raw)
  To: Dave Chinner; +Cc: fstests, willy

On Fri, May 04, 2018 at 10:05:09AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Everyone who starts using fstests runs "check" without parameters,
> and then has problems with it running dangerous tests. most people
> just want fstests to act as a regression test suite, not a fuzzer or
> exercise known crash conditions. Hence make the default behaviour to
> be "run the auto group" rather than "run every test".
> 
> To enable people to run all tests easily (if they really want to)
> add a special group keyword named "all". This wildcard will trigger
> selecting all the tests in fstests as per the original "check
> without parameters" behaviour.
> 
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> ---
>  check | 34 +++++++++++++++++++++-------------
>  1 file changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/check b/check
> index 84a35e6724ab..170004d4b2a2 100755
> --- a/check
> +++ b/check
> @@ -102,6 +102,8 @@ a test file name match pattern (e.g. xfs/*).
>  group argument is either a name of a tests group to collect from all
>  the test dirs (e.g. quick) or a name of a tests group to collect from
>  a specific tests dir in the form of <test dir>/<group name> (e.g. xfs/quick).
> +If you want to run all the tests in teh test suite, use "-g all" to specify all

...the test suite... (unless you really mean the rock band :P)

Looks ok otherwise,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> +groups.
>  
>  exclude_file argument refers to a name of a file inside each test directory.
>  for every test dir where this file is found, the listed test names are
> @@ -220,22 +222,24 @@ _prepare_test_list()
>  	fi
>  
>  	# Specified groups to include
> -	for group in $GROUP_LIST; do
> -		list=$(get_group_list $group)
> -		if [ -z "$list" ]; then
> -			echo "Group \"$group\" is empty or not defined?"
> -			exit 1
> -		fi
> +	# Note that the CLI processing adds a leading space to the first group
> +	# parameter, so we have to catch that here checking for "all"
> +	if ! $have_test_arg && [ "$GROUP_LIST" == " all" ]; then
> +		# no test numbers, do everything
> +		get_all_tests
> +	else
> +		for group in $GROUP_LIST; do
> +			list=$(get_group_list $group)
> +			if [ -z "$list" ]; then
> +				echo "Group \"$group\" is empty or not defined?"
> +				exit 1
> +			fi
>  
> -		for t in $list; do
> -			grep -s "^$t\$" $tmp.list >/dev/null || \
> +			for t in $list; do
> +				grep -s "^$t\$" $tmp.list >/dev/null || \
>  							echo "$t" >>$tmp.list
> +			done
>  		done
> -	done
> -
> -	if ! $have_test_arg && [ -z "$GROUP_LIST" ]; then
> -		# no test numbers, do everything
> -		get_all_tests
>  	fi
>  
>  	# Specified groups to exclude
> @@ -364,6 +368,10 @@ if $have_test_arg; then
>  
>  		shift
>  	done
> +elif [ -z "$GROUP_LIST" ]; then
> +	# default group list is the auto group. If any other group or test is
> +	# specified, we use that instead.
> +	GROUP_LIST="auto"
>  fi
>  
>  # we need common/rc
> -- 
> 2.17.0
> 
> --
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] check: run auto test group by default
  2018-05-04  0:05 [PATCH] check: run auto test group by default Dave Chinner
                   ` (2 preceding siblings ...)
  2018-05-04  3:50 ` Darrick J. Wong
@ 2018-05-04  5:55 ` Amir Goldstein
  2018-05-04  7:06   ` Eryu Guan
  3 siblings, 1 reply; 7+ messages in thread
From: Amir Goldstein @ 2018-05-04  5:55 UTC (permalink / raw)
  To: Dave Chinner; +Cc: fstests, willy

On Fri, May 4, 2018 at 3:05 AM, Dave Chinner <david@fromorbit.com> wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Everyone who starts using fstests runs "check" without parameters,
> and then has problems with it running dangerous tests. most people
> just want fstests to act as a regression test suite, not a fuzzer or
> exercise known crash conditions. Hence make the default behaviour to
> be "run the auto group" rather than "run every test".
>
> To enable people to run all tests easily (if they really want to)
> add a special group keyword named "all". This wildcard will trigger
> selecting all the tests in fstests as per the original "check
> without parameters" behaviour.
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>

It's great that we are being more welcoming to xfstests newcomers!
in that note, please update README.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] check: run auto test group by default
  2018-05-04  5:55 ` Amir Goldstein
@ 2018-05-04  7:06   ` Eryu Guan
  0 siblings, 0 replies; 7+ messages in thread
From: Eryu Guan @ 2018-05-04  7:06 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Dave Chinner, fstests, willy

On Fri, May 04, 2018 at 08:55:37AM +0300, Amir Goldstein wrote:
> On Fri, May 4, 2018 at 3:05 AM, Dave Chinner <david@fromorbit.com> wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > Everyone who starts using fstests runs "check" without parameters,
> > and then has problems with it running dangerous tests. most people
> > just want fstests to act as a regression test suite, not a fuzzer or
> > exercise known crash conditions. Hence make the default behaviour to
> > be "run the auto group" rather than "run every test".
> >
> > To enable people to run all tests easily (if they really want to)
> > add a special group keyword named "all". This wildcard will trigger
> > selecting all the tests in fstests as per the original "check
> > without parameters" behaviour.
> >
> > Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> 
> It's great that we are being more welcoming to xfstests newcomers!
> in that note, please update README.

Yeah, a line in README to describe this would be great.

Thanks,
Eryu

> 
> Thanks,
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] check: run auto test group by default
  2018-05-04  3:50 ` Darrick J. Wong
@ 2018-05-04 14:58   ` Eryu Guan
  0 siblings, 0 replies; 7+ messages in thread
From: Eryu Guan @ 2018-05-04 14:58 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Dave Chinner, fstests, willy

On Thu, May 03, 2018 at 08:50:39PM -0700, Darrick J. Wong wrote:
> On Fri, May 04, 2018 at 10:05:09AM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Everyone who starts using fstests runs "check" without parameters,
> > and then has problems with it running dangerous tests. most people
> > just want fstests to act as a regression test suite, not a fuzzer or
> > exercise known crash conditions. Hence make the default behaviour to
> > be "run the auto group" rather than "run every test".
> > 
> > To enable people to run all tests easily (if they really want to)
> > add a special group keyword named "all". This wildcard will trigger
> > selecting all the tests in fstests as per the original "check
> > without parameters" behaviour.
> > 
> > Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> > ---
> >  check | 34 +++++++++++++++++++++-------------
> >  1 file changed, 21 insertions(+), 13 deletions(-)
> > 
> > diff --git a/check b/check
> > index 84a35e6724ab..170004d4b2a2 100755
> > --- a/check
> > +++ b/check
> > @@ -102,6 +102,8 @@ a test file name match pattern (e.g. xfs/*).
> >  group argument is either a name of a tests group to collect from all
> >  the test dirs (e.g. quick) or a name of a tests group to collect from
> >  a specific tests dir in the form of <test dir>/<group name> (e.g. xfs/quick).
> > +If you want to run all the tests in teh test suite, use "-g all" to specify all
> 
> ...the test suite... (unless you really mean the rock band :P)

Fixed on commit.

> 
> Looks ok otherwise,
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks.

Eryu

> 
> --D
> 
> > +groups.
> >  
> >  exclude_file argument refers to a name of a file inside each test directory.
> >  for every test dir where this file is found, the listed test names are
> > @@ -220,22 +222,24 @@ _prepare_test_list()
> >  	fi
> >  
> >  	# Specified groups to include
> > -	for group in $GROUP_LIST; do
> > -		list=$(get_group_list $group)
> > -		if [ -z "$list" ]; then
> > -			echo "Group \"$group\" is empty or not defined?"
> > -			exit 1
> > -		fi
> > +	# Note that the CLI processing adds a leading space to the first group
> > +	# parameter, so we have to catch that here checking for "all"
> > +	if ! $have_test_arg && [ "$GROUP_LIST" == " all" ]; then
> > +		# no test numbers, do everything
> > +		get_all_tests
> > +	else
> > +		for group in $GROUP_LIST; do
> > +			list=$(get_group_list $group)
> > +			if [ -z "$list" ]; then
> > +				echo "Group \"$group\" is empty or not defined?"
> > +				exit 1
> > +			fi
> >  
> > -		for t in $list; do
> > -			grep -s "^$t\$" $tmp.list >/dev/null || \
> > +			for t in $list; do
> > +				grep -s "^$t\$" $tmp.list >/dev/null || \
> >  							echo "$t" >>$tmp.list
> > +			done
> >  		done
> > -	done
> > -
> > -	if ! $have_test_arg && [ -z "$GROUP_LIST" ]; then
> > -		# no test numbers, do everything
> > -		get_all_tests
> >  	fi
> >  
> >  	# Specified groups to exclude
> > @@ -364,6 +368,10 @@ if $have_test_arg; then
> >  
> >  		shift
> >  	done
> > +elif [ -z "$GROUP_LIST" ]; then
> > +	# default group list is the auto group. If any other group or test is
> > +	# specified, we use that instead.
> > +	GROUP_LIST="auto"
> >  fi
> >  
> >  # we need common/rc
> > -- 
> > 2.17.0
> > 
> > --
> > 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
> --
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-05-04 14:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-04  0:05 [PATCH] check: run auto test group by default Dave Chinner
2018-05-04  3:06 ` Matthew Wilcox
2018-05-04  3:13 ` Eric Sandeen
2018-05-04  3:50 ` Darrick J. Wong
2018-05-04 14:58   ` Eryu Guan
2018-05-04  5:55 ` Amir Goldstein
2018-05-04  7:06   ` Eryu Guan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.