| Message ID | 1470382393-24209-1-git-send-email-sjitindarsingh@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Aug 05, 2016 at 05:33:10PM +1000, Suraj Jitindar Singh wrote: > Invoking run_tests.sh without the -g parameter will by default run all of > the tests for a given architecture. This patch series will add a test which > has the ability to bring down the host and thus it might be nice if we > double check that the user actually wants to run that test instead of > them unknowingly bringing down a machine they might not want to. > > In order to do this add the option for a tests' group parameter in > unittests.cfg to be set as "nodefault" on order to indicate that > it shouldn't be run be default. Modify runtime.bash such that if one of > these tests is encountered a message will be printed to the user to > indicate that the task was skipped and with instructions on how to run > the test on it's own. > > This allows a user to confirm that they want to run a test which has been > marked as not to be run by default for whatever reason by the creator. > Existing functionality will be preserved and new tests can choose any > group other than "nodefault" if they want to be run by default. > > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com> > --- > arm/unittests.cfg | 3 +++ > powerpc/unittests.cfg | 3 +++ > scripts/runtime.bash | 10 ++++++++++ > x86/unittests.cfg | 3 +++ > 4 files changed, 19 insertions(+) > > I was going to have the long error message gated behind > [ $verbose == "yes" ] but this means that when that test is run standalone > it will skip without any obvious indication as to why. > Thus IMO it's better to have the message printed regardless. > > diff --git a/arm/unittests.cfg b/arm/unittests.cfg > index ffd12e5..3f6fa45 100644 > --- a/arm/unittests.cfg > +++ b/arm/unittests.cfg > @@ -12,6 +12,9 @@ > # # specific to only one. > # groups = <group_name1> <group_name2> ... # Used to identify test cases > # # with run_tests -g ... > +# # Specify group_name=nodefault > +# # to have test not run by > +# # default > # accel = kvm|tcg # Optionally specify if test must run with > # # kvm or tcg. If not specified, then kvm will > # # be used when available. > diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg > index ed4fdbe..0098cb6 100644 > --- a/powerpc/unittests.cfg > +++ b/powerpc/unittests.cfg > @@ -12,6 +12,9 @@ > # # specific to only one. > # groups = <group_name1> <group_name2> ... # Used to identify test cases > # # with run_tests -g ... > +# # Specify group_name=nodefault > +# # to have test not run by > +# # default > # accel = kvm|tcg # Optionally specify if test must run with > # # kvm or tcg. If not specified, then kvm will > # # be used when available. > diff --git a/scripts/runtime.bash b/scripts/runtime.bash > index 0503cf0..6bf28fb 100644 > --- a/scripts/runtime.bash > +++ b/scripts/runtime.bash > @@ -52,6 +52,16 @@ function run() > return > fi > > + if grep -q "nodefault" <<<$groups && ! grep -qw "$only_group" <<<$groups; then I think we want if [ -z "$only_group" ] && grep -qw "nodefault" <<<$groups The condition above already checks that if $only_group is given that it's in $groups. Also, please add -w to the grep in that condition above. > + echo -e "`SKIP` $testname\n" \ > + "Test $testname marked as not to be run by default,\n" \ > + "please ensure that you actually want to run this test\n" \ > + "To run this using run_tests.sh append \"-g $groups\"\n" \ > + "To run this standalone set the only_group parameter\n" \ > + "\"only_group=$groups tests/$testname\"" We prefer one line summaries be output here. How about just adding "(manual run only - host may crash)" or some such, to the SKIP line. As for the 'only_group=$groups tests/$testname' standalone instructions. I think mkstandalone should be modified to check for nodefault and output a message saying only continue if you're sure, and then wait for input from the user to continue. > + return; > + fi > + > if [ -n "$arch" ] && [ "$arch" != "$ARCH" ]; then > echo "`SKIP` $1 ($arch only)" > return 2 > diff --git a/x86/unittests.cfg b/x86/unittests.cfg > index 60747cf..4a1f74e 100644 > --- a/x86/unittests.cfg > +++ b/x86/unittests.cfg > @@ -12,6 +12,9 @@ > # # specific to only one. > # groups = <group_name1> <group_name2> ... # Used to identify test cases > # # with run_tests -g ... > +# # Specify group_name=nodefault > +# # to have test not run by > +# # default > # accel = kvm|tcg # Optionally specify if test must run with > # # kvm or tcg. If not specified, then kvm will > # # be used when available. > -- > 2.5.5 Thanks, drew -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2016-08-05 at 10:18 +0200, Andrew Jones wrote: > On Fri, Aug 05, 2016 at 05:33:10PM +1000, Suraj Jitindar Singh wrote: > > > > Invoking run_tests.sh without the -g parameter will by default run > > all of > > the tests for a given architecture. This patch series will add a > > test which > > has the ability to bring down the host and thus it might be nice if > > we > > double check that the user actually wants to run that test instead > > of > > them unknowingly bringing down a machine they might not want to. > > > > In order to do this add the option for a tests' group parameter in > > unittests.cfg to be set as "nodefault" on order to indicate that > > it shouldn't be run be default. Modify runtime.bash such that if > > one of > > these tests is encountered a message will be printed to the user to > > indicate that the task was skipped and with instructions on how to > > run > > the test on it's own. > > > > This allows a user to confirm that they want to run a test which > > has been > > marked as not to be run by default for whatever reason by the > > creator. > > Existing functionality will be preserved and new tests can choose > > any > > group other than "nodefault" if they want to be run by default. > > > > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com> > > --- > > arm/unittests.cfg | 3 +++ > > powerpc/unittests.cfg | 3 +++ > > scripts/runtime.bash | 10 ++++++++++ > > x86/unittests.cfg | 3 +++ > > 4 files changed, 19 insertions(+) > > > > I was going to have the long error message gated behind > > [ $verbose == "yes" ] but this means that when that test is run > > standalone > > it will skip without any obvious indication as to why. > > Thus IMO it's better to have the message printed regardless. > > > > diff --git a/arm/unittests.cfg b/arm/unittests.cfg > > index ffd12e5..3f6fa45 100644 > > --- a/arm/unittests.cfg > > +++ b/arm/unittests.cfg > > @@ -12,6 +12,9 @@ > > # # specific to only one. > > # groups = <group_name1> <group_name2> ... # Used to > > identify test cases > > # # with run_tests > > -g ... > > +# # Specify > > group_name=nodefault > > +# # to have test > > not run by > > +# # default > > # accel = kvm|tcg # Optionally specify if test must > > run with > > # # kvm or tcg. If not specified, > > then kvm will > > # # be used when available. > > diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg > > index ed4fdbe..0098cb6 100644 > > --- a/powerpc/unittests.cfg > > +++ b/powerpc/unittests.cfg > > @@ -12,6 +12,9 @@ > > # # specific to only one. > > # groups = <group_name1> <group_name2> ... # Used to > > identify test cases > > # # with run_tests > > -g ... > > +# # Specify > > group_name=nodefault > > +# # to have test > > not run by > > +# # default > > # accel = kvm|tcg # Optionally specify if test must > > run with > > # # kvm or tcg. If not specified, > > then kvm will > > # # be used when available. > > diff --git a/scripts/runtime.bash b/scripts/runtime.bash > > index 0503cf0..6bf28fb 100644 > > --- a/scripts/runtime.bash > > +++ b/scripts/runtime.bash > > @@ -52,6 +52,16 @@ function run() > > return > > fi > > > > + if grep -q "nodefault" <<<$groups && ! grep -qw "$only_group" > > <<<$groups; then > I think we want if [ -z "$only_group" ] && grep -qw "nodefault" > <<<$groups > The condition above already checks that if $only_group is given that > it's > in $groups. Also, please add -w to the grep in that condition above. That would make more sense, I'll change this. > > > > > + echo -e "`SKIP` $testname\n" \ > > + "Test $testname marked as not to be run by default,\n" > > \ > > + "please ensure that you actually want to run this > > test\n" \ > > + "To run this using run_tests.sh append \"-g > > $groups\"\n" \ > > + "To run this standalone set the only_group > > parameter\n" \ > > + "\"only_group=$groups tests/$testname\"" > We prefer one line summaries be output here. How about just adding > "(manual run only - host may crash)" or some such, to the SKIP line. Ok, I'll make it something like "test marked manual run only" as this may have been done for reasons other than the test can bring down the host. > As for the 'only_group=$groups tests/$testname' standalone > instructions. > I think mkstandalone should be modified to check for nodefault and > output a message saying only continue if you're sure, and then wait > for > input from the user to continue. That's a more logical option, this was just the easiest. I'll look into doing something like this. When you say mkstandalone should be modified do you mean when the user runs "make standalone" to build the tests they should be prompted or when they actually invoke the test they are asked to confirm that they actually want to run it? Having the confirmation step on actual test invokation makes more sense IMO. > > > > > + return; > > + fi > > + > > if [ -n "$arch" ] && [ "$arch" != "$ARCH" ]; then > > echo "`SKIP` $1 ($arch only)" > > return 2 > > diff --git a/x86/unittests.cfg b/x86/unittests.cfg > > index 60747cf..4a1f74e 100644 > > --- a/x86/unittests.cfg > > +++ b/x86/unittests.cfg > > @@ -12,6 +12,9 @@ > > # # specific to only one. > > # groups = <group_name1> <group_name2> ... # Used to > > identify test cases > > # # with run_tests > > -g ... > > +# # Specify > > group_name=nodefault > > +# # to have test > > not run by > > +# # default > > # accel = kvm|tcg # Optionally specify if test must > > run with > > # # kvm or tcg. If not specified, > > then kvm will > > # # be used when available. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 08, 2016 at 01:47:07PM +1000, Suraj Jitindar Singh wrote: > On Fri, 2016-08-05 at 10:18 +0200, Andrew Jones wrote: > > On Fri, Aug 05, 2016 at 05:33:10PM +1000, Suraj Jitindar Singh wrote: > > > > > > Invoking run_tests.sh without the -g parameter will by default run > > > all of > > > the tests for a given architecture. This patch series will add a > > > test which > > > has the ability to bring down the host and thus it might be nice if > > > we > > > double check that the user actually wants to run that test instead > > > of > > > them unknowingly bringing down a machine they might not want to. > > > > > > In order to do this add the option for a tests' group parameter in > > > unittests.cfg to be set as "nodefault" on order to indicate that > > > it shouldn't be run be default. Modify runtime.bash such that if > > > one of > > > these tests is encountered a message will be printed to the user to > > > indicate that the task was skipped and with instructions on how to > > > run > > > the test on it's own. > > > > > > This allows a user to confirm that they want to run a test which > > > has been > > > marked as not to be run by default for whatever reason by the > > > creator. > > > Existing functionality will be preserved and new tests can choose > > > any > > > group other than "nodefault" if they want to be run by default. > > > > > > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com> > > > --- > > > arm/unittests.cfg | 3 +++ > > > powerpc/unittests.cfg | 3 +++ > > > scripts/runtime.bash | 10 ++++++++++ > > > x86/unittests.cfg | 3 +++ > > > 4 files changed, 19 insertions(+) > > > > > > I was going to have the long error message gated behind > > > [ $verbose == "yes" ] but this means that when that test is run > > > standalone > > > it will skip without any obvious indication as to why. > > > Thus IMO it's better to have the message printed regardless. > > > > > > diff --git a/arm/unittests.cfg b/arm/unittests.cfg > > > index ffd12e5..3f6fa45 100644 > > > --- a/arm/unittests.cfg > > > +++ b/arm/unittests.cfg > > > @@ -12,6 +12,9 @@ > > > # # specific to only one. > > > # groups = <group_name1> <group_name2> ... # Used to > > > identify test cases > > > # # with run_tests > > > -g ... > > > +# # Specify > > > group_name=nodefault > > > +# # to have test > > > not run by > > > +# # default > > > # accel = kvm|tcg # Optionally specify if test must > > > run with > > > # # kvm or tcg. If not specified, > > > then kvm will > > > # # be used when available. > > > diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg > > > index ed4fdbe..0098cb6 100644 > > > --- a/powerpc/unittests.cfg > > > +++ b/powerpc/unittests.cfg > > > @@ -12,6 +12,9 @@ > > > # # specific to only one. > > > # groups = <group_name1> <group_name2> ... # Used to > > > identify test cases > > > # # with run_tests > > > -g ... > > > +# # Specify > > > group_name=nodefault > > > +# # to have test > > > not run by > > > +# # default > > > # accel = kvm|tcg # Optionally specify if test must > > > run with > > > # # kvm or tcg. If not specified, > > > then kvm will > > > # # be used when available. > > > diff --git a/scripts/runtime.bash b/scripts/runtime.bash > > > index 0503cf0..6bf28fb 100644 > > > --- a/scripts/runtime.bash > > > +++ b/scripts/runtime.bash > > > @@ -52,6 +52,16 @@ function run() > > > return > > > fi > > > > > > + if grep -q "nodefault" <<<$groups && ! grep -qw "$only_group" > > > <<<$groups; then > > I think we want if [ -z "$only_group" ] && grep -qw "nodefault" > > <<<$groups > > The condition above already checks that if $only_group is given that > > it's > > in $groups. Also, please add -w to the grep in that condition above. > That would make more sense, I'll change this. > > > > > > > > + echo -e "`SKIP` $testname\n" \ > > > + "Test $testname marked as not to be run by default,\n" > > > \ > > > + "please ensure that you actually want to run this > > > test\n" \ > > > + "To run this using run_tests.sh append \"-g > > > $groups\"\n" \ > > > + "To run this standalone set the only_group > > > parameter\n" \ > > > + "\"only_group=$groups tests/$testname\"" > > We prefer one line summaries be output here. How about just adding > > "(manual run only - host may crash)" or some such, to the SKIP line. > Ok, I'll make it something like "test marked manual run only" as > this > may have been done for reasons other than the test can bring > down the > host. > > As for the 'only_group=$groups tests/$testname' standalone > > instructions. > > I think mkstandalone should be modified to check for nodefault and > > output a message saying only continue if you're sure, and then wait > > for > > input from the user to continue. > That's a more logical option, this was just the easiest. I'll look > into doing something like this. > When you say mkstandalone should be modified do you mean when > the user runs "make standalone" to build the tests they should be > prompted or when they actually invoke the test they are asked > to confirm that they actually want to run it? Having the > confirmation step on actual test invokation makes more sense IMO. Sorry for the slow response. I've been on vacation. Yes, I meant the later, when the user runs the test she'll get prompted to continue for 'nodefault' type tests. Thanks, drew > > > > > > > > + return; > > > + fi > > > + > > > if [ -n "$arch" ] && [ "$arch" != "$ARCH" ]; then > > > echo "`SKIP` $1 ($arch only)" > > > return 2 > > > diff --git a/x86/unittests.cfg b/x86/unittests.cfg > > > index 60747cf..4a1f74e 100644 > > > --- a/x86/unittests.cfg > > > +++ b/x86/unittests.cfg > > > @@ -12,6 +12,9 @@ > > > # # specific to only one. > > > # groups = <group_name1> <group_name2> ... # Used to > > > identify test cases > > > # # with run_tests > > > -g ... > > > +# # Specify > > > group_name=nodefault > > > +# # to have test > > > not run by > > > +# # default > > > # accel = kvm|tcg # Optionally specify if test must > > > run with > > > # # kvm or tcg. If not specified, > > > then kvm will > > > # # be used when available. > -- > To unsubscribe from this list: send the line "unsubscribe kvm" 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 kvm" 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/arm/unittests.cfg b/arm/unittests.cfg index ffd12e5..3f6fa45 100644 --- a/arm/unittests.cfg +++ b/arm/unittests.cfg @@ -12,6 +12,9 @@ # # specific to only one. # groups = <group_name1> <group_name2> ... # Used to identify test cases # # with run_tests -g ... +# # Specify group_name=nodefault +# # to have test not run by +# # default # accel = kvm|tcg # Optionally specify if test must run with # # kvm or tcg. If not specified, then kvm will # # be used when available. diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg index ed4fdbe..0098cb6 100644 --- a/powerpc/unittests.cfg +++ b/powerpc/unittests.cfg @@ -12,6 +12,9 @@ # # specific to only one. # groups = <group_name1> <group_name2> ... # Used to identify test cases # # with run_tests -g ... +# # Specify group_name=nodefault +# # to have test not run by +# # default # accel = kvm|tcg # Optionally specify if test must run with # # kvm or tcg. If not specified, then kvm will # # be used when available. diff --git a/scripts/runtime.bash b/scripts/runtime.bash index 0503cf0..6bf28fb 100644 --- a/scripts/runtime.bash +++ b/scripts/runtime.bash @@ -52,6 +52,16 @@ function run() return fi + if grep -q "nodefault" <<<$groups && ! grep -qw "$only_group" <<<$groups; then + echo -e "`SKIP` $testname\n" \ + "Test $testname marked as not to be run by default,\n" \ + "please ensure that you actually want to run this test\n" \ + "To run this using run_tests.sh append \"-g $groups\"\n" \ + "To run this standalone set the only_group parameter\n" \ + "\"only_group=$groups tests/$testname\"" + return; + fi + if [ -n "$arch" ] && [ "$arch" != "$ARCH" ]; then echo "`SKIP` $1 ($arch only)" return 2 diff --git a/x86/unittests.cfg b/x86/unittests.cfg index 60747cf..4a1f74e 100644 --- a/x86/unittests.cfg +++ b/x86/unittests.cfg @@ -12,6 +12,9 @@ # # specific to only one. # groups = <group_name1> <group_name2> ... # Used to identify test cases # # with run_tests -g ... +# # Specify group_name=nodefault +# # to have test not run by +# # default # accel = kvm|tcg # Optionally specify if test must run with # # kvm or tcg. If not specified, then kvm will # # be used when available.
Invoking run_tests.sh without the -g parameter will by default run all of the tests for a given architecture. This patch series will add a test which has the ability to bring down the host and thus it might be nice if we double check that the user actually wants to run that test instead of them unknowingly bringing down a machine they might not want to. In order to do this add the option for a tests' group parameter in unittests.cfg to be set as "nodefault" on order to indicate that it shouldn't be run be default. Modify runtime.bash such that if one of these tests is encountered a message will be printed to the user to indicate that the task was skipped and with instructions on how to run the test on it's own. This allows a user to confirm that they want to run a test which has been marked as not to be run by default for whatever reason by the creator. Existing functionality will be preserved and new tests can choose any group other than "nodefault" if they want to be run by default. Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com> --- arm/unittests.cfg | 3 +++ powerpc/unittests.cfg | 3 +++ scripts/runtime.bash | 10 ++++++++++ x86/unittests.cfg | 3 +++ 4 files changed, 19 insertions(+) I was going to have the long error message gated behind [ $verbose == "yes" ] but this means that when that test is run standalone it will skip without any obvious indication as to why. Thus IMO it's better to have the message printed regardless.