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@xxxxxxxxx> > > > --- > > > 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@xxxxxxxxxxxxxxx > 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
