On Wed, 2016-08-10 at 15:22 +0200, Radim Krčmář wrote:
> 2016-08-10 11:59+1000, Suraj Jitindar Singh:
> >
> > diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> > @@ -74,6 +74,27 @@ generate_test ()
> >
> > cat scripts/runtime.bash
> >
> > + if grep -qw "nodefault" <<<${args[1]}; then
> > + echo -e "while true; do\n"\
> > + "\tread -p \"Test marked as not to be run
> > by default,"\
> > + "are you sure (Y/N)? \" response\n"\
> > + "\tcase \$response in\n"\
> > + "\t\t\"Y\" | \"y\" | \"Yes\" |
> > \"yes\")\n"\
> > + "\t\t\tbreak\n"\
> > + "\t\t\t;;\n"\
> > + "\t\t\"N\" | \"n\" | \"No\" | \"no\")\n"\
> > + "\t\t\t;&\n"\
> > + "\t\t\"q\" | \"quit\" | \"exit\")\n"\
> > + "\t\t\texit\n"\
> > + "\t\t\t;;\n"\
> > + "\t\t*)\n"\
> > + "\t\t\techo Please select Y or N\n"\
> > + "\t\t\t;;\n"\
> > + "\tesac\n"\
> > + "done"
> Uff, this is hard to read.
>
> We do not care much about readability of the standalone script
> itself,
> but the source code should be. It doesn't have to have be that fancy
> with user input either:
>
> echo 'read -p "$question? (y/N)' response
> echo 'case $response in'
> echo ' Y|y|Yes|yes) break;;'
> echo ' *) exit;;
> echo 'esac'
>
> It's still ugly, what about adding a function to
> scripts/runtime.bash?
> More on that below.
>
> >
> > + echo "standalone=\"true\""
> We already have $STANDALONE,
>
> echo "export STANDALONE=yes"
>
> >
> > diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> > @@ -48,10 +48,16 @@ function run()
> > return
> > fi
> >
> > - if [ -n "$only_group" ] && ! grep -q "$only_group" <<<$groups;
> > then
> > + if [ -n "$only_group" ] && ! grep -qw "$only_group"
> > <<<$groups; then
> > return
> > fi
> >
> > + if [ -z "$only_group" ] && grep -qw "nodefault" <<<$groups &&
> > + ([ -z $standalone ] || [ $standalone != "true" ]);
> > then
> Continuing the idea about a function: This can be replaced with
>
> if [ -z "$only_group" ] && grep -qw "nodefault" <<<$groups &&
> skip_nodefault;
>
> with skip_nodefault defined earlier; It is not a horrible loss to
> load
> more code in the normal run,
>
> skip_nodefault () {
> [ "$STANDALONE" != yes ] && return true
>
> # code ask the question and handle responses -- can be a
> fancier
> # now, that it actually is readable
> }
>
> That said, I am not a huge fan of user interaction in tests ...
> What is the targeted use-case?
The idea was basically to add the option to mark a test as not to
be run by default when invoking run_tests.sh. It was then suggested
on a previous version of this series that when invoked as a standalone
test the user be prompted to confirm that they actually want to
run the test.
Since there may be tests which can have a detrimental effect on the
host system or some other unintended side effect I thought it better to
require the user specifically invoke them.
>
> The user has already specifically called this test, ./host_killer, so
> asking for confirmation is implying that the user is a monkey.
>
> If the test was scripted, then we forced something like
> `yes | ./host_killer`.
I agree in hindsight that it doesn't make much sense to have the user
confirm that they want to run a test that they have specifically
invoked. That being said it's possible that someone running it may not
know that it has potentially negative effects on the host.
I think it might be better to have tests in the nodefault group require
explicit selection by the "-g" parameter when running through
run_tests.sh (current effect of series), while when a test is run
standalone just run it without any additional user input (different to
current operation) and assume the user knows what they are doing. Do
you agree with this?
>
> >
> > + echo -e "`SKIP` $testname - (test marked as manual run
> > only)"
> Please remove the whitespaced dash " - " from output.
>
Will Fix
> Thanks.
Thanks for the comments
--
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
