From mboxrd@z Thu Jan  1 00:00:00 1970
From: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Subject: Re: [kvm-unit-tests PATCH V4 1/5] scripts/runtime: Add ability to
 mark test as don't run by default
Date: Thu, 18 Aug 2016 14:46:09 +1000
Message-ID: <1471495569.2138.21.camel@gmail.com>
References: <1471416538-14088-1-git-send-email-sjitindarsingh@gmail.com>
	 <20160817150152.GB22826@potion>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
Cc: kvm@vger.kernel.org, pbonzini@redhat.com, kvm-ppc@vger.kernel.org,
	lvivier@redhat.com, thuth@redhat.com, drjones@redhat.com
To: Radim =?UTF-8?Q?Kr=C4=8Dm=C3=A1=C5=99?= <rkrcmar@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-pa0-f68.google.com ([209.85.220.68]:35851 "EHLO
	mail-pa0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1750720AbcHRE6e (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 18 Aug 2016 00:58:34 -0400
In-Reply-To: <20160817150152.GB22826@potion>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Wed, 2016-08-17 at 17:01 +0200, Radim Krčmář wrote:
> 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
Thanks, I'll respin and address these
> 
> 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
> > +}
> > +