2016-08-12 12:00+0200, Andrew Jones:
> On Fri, Aug 12, 2016 at 04:13:13PM +1000, Suraj Jitindar Singh wrote:
>> 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?
>
> I disagree. I like the extra protection. The name of the test won't
> be "host-killer", it'll be something like "test-obscure-named-feature".
> The point of standalone tests is to be able to pass them around easily
> and store them for later use. So it's quite likely that the person who
> stores it won't be the person who runs it (or the person who stores it
> will forget what it does by the time they run it) Anybody who wants to
> avoid the prompt can simply wrap the standalone script in another one
>
> cat <<EOF > set-trap-for-unsuspecting-users
> #/bin/bash
> yes | ./test-obscure-named-feature
> EOF
Ok, experience with `yum` made me tolerant. :)
I would go with the check inside scripts/runtime.bash then.
> We could also add a couple standard options to standalone tests,
> -h (help - output what the test does, warn about crashing hosts, etc.)
Sounds nice.
Could also work with `./run_tests.sh -h` to print them all.
> -y (yes - say yes at any prompts)
What about adding a "-g $group" option to standalone tests instead.?
We could then use
for test in tests/*; do $test -g $group; done
to run the same tests as
./run_test.sh -g $group
> -h would take its text from the unittests.cfg file (we'd add a new
> unit test property called 'help' there)
--
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
