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

["Radim Krčmář" <rkrcmar@redhat.com>]

2016-08-17 16:48+1000, Suraj Jitindar Singh:
> 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 include "nodefault" on order to indicate that it shouldn't
> be run be default.
> 
> When tests are invoked via run_tests.sh those with the nodefault group
> parameter will be skipped unless explicitly specified by the "-g" command
> line option. When tests with the nodefault group parameter are built and
> run standalone the user will be prompted on invocation to confirm that
> they actually want to run the test.
> 
> This allows a developer to mark a test as having potentially adverse
> effects and thus requires an extra level of confirmation from the user
> before they are invoked. 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>
> ---

I have only nits, so

Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>

> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> @@ -32,6 +32,25 @@ get_cmdline()
> +skip_nodefault()
> +{
> +    [ "$STANDALONE" != "yes" ] && return 0
> +
> +    while true; do
> +        read -p "Test marked not to be run by default, are you sure (Y/N)? " yn

"y/N" would help to understand the default with "".

> +        case $yn in
> +            "Y" | "y" | "Yes" | "yes")
> +                return 1
> +                ;;
> +            "" | "N" | "n" | "No" | "no" | "q" | "quit" | "exit")
> +                return 0
> +                ;;
> +            *)
> +                ;;

The "*) ;;" case doesn't have to be there.

> +        esac
> +    done
> +}
> +