Re: [kvm-unit-tests PATCH V3 1/5] scripts/runtime: Add ability to mark test as don't run by default

[Suraj Jitindar Singh <sjitindarsingh@gmail.com>]

On Tue, 2016-08-16 at 18:03 +0200, Radim Krčmář wrote:
> 2016-08-16 14:00+0200, Andrew Jones:
> > 
> > On Tue, Aug 16, 2016 at 05:18:11PM +1000, Suraj Jitindar Singh
> > wrote:
> > > 
> > > diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> > > @@ -32,6 +32,26 @@ get_cmdline()
> > > +skip_nodefault()
> > > +{
> > > +    while true; do
> > > +        read -p "Test marked not to be run by default, are you
> > > sure (Y/N)? " yn
> I'd write "run this test" instead of "are you sure", or something
> similar for the question.  The user can be sure that the has is
> marked
> with nodefault. ;)
I'll go with "run this test?"
> 
> > 
> > > 
> > > +        case $yn in
> > > +            "Y" | "y" | "Yes" | "yes")
> > What about "YES" :-)
> And exclamation marks!
> 
> "YES!!!"
> 
> > 
> > Actually, I'd just accept 'Y' for yes, and nothing else, like the
> > prompt says.
> NO.  If it is only one value, then make it "y".
> motto: saving the Earth, one shift at a time.
> 
> This kind of user interface usually accepts at least "[yY]|[yY]es"
> ...
> users will already be pissed that they have to input something and
> denying a perfectly logical "yes" (which is what "y" stands for) is
> going too overboard, IMO.
> 
> > 
> > And, instead of looping for valid input, all other input can just
> > mean no.
> "y/N" is the convention for writing a bool question that defaults to
> no.
> I'd accept "" (just enter) as the default and then, looping isn't
> unexpected and user already typed some crap in that case, so they
> probably want to answer the question without having to run the
> command
> again.
I'd rather keep the "[yY]|[yY]es" options it doesn't really make sense
not to.
I can see the argument for having all other input default to no. You
really see a mix of the two in the wild where sometimes junk will
default to no and sometimes it will loop and ask again.
I tend to think that most of the time when it doesn't match one of the
options it's going to be because of a mis-type by the user and they'd
probably prefer that it looped than have to run the command again.
> 
> > 
> > > 
> > > +                return 1
> > > +                ;;
> > > +            "N" | "n" | "No" | "no" | "q" | "quit" | "exit")
> > We should output something when the answer is 'no' like "User
> > aborted",
> > or whatever.
I don't really see this as being necessary, I guess there could be some
ambiguity as to what has happened but I think it's pretty obvious that
the test has aborted.
I'll change it so that the skip message is printed.
> > 
> > > 
> > > +                exit
> Wouldn't "return 1" and the SKIP message be enough?
Yeah if I have return 0 here (because bash things) then it'll print the
skip message and abort the test.
> 
> > 
> > > 
> > > +                ;;
> > > +            *)
> > > +                echo Please select Y or N
> Just asking the question again is more common -- it's not hard to
> figure
> out that the answer was not accepted.
Ok, I've seen places where the options are specified like this but will
change it to just have the same question again.