diff mbox

[KVM-AUTOTEST] Check exit status of custom install script and fail if script failed.

Message ID 1242851931-8492-2-git-send-email-mburns@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mike Burns May 20, 2009, 8:38 p.m. UTC
Signed-off-by: Mike Burns <mburns@redhat.com>
---
 client/tests/kvm_runtest_2/kvm_install.py |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

Comments

Avi Kivity May 24, 2009, 2:48 p.m. UTC | #1
Mike Burns wrote:
> Signed-off-by: Mike Burns <mburns@redhat.com>
> ---
>  client/tests/kvm_runtest_2/kvm_install.py |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/client/tests/kvm_runtest_2/kvm_install.py b/client/tests/kvm_runtest_2/kvm_install.py
> index ebd8b7d..392ef0c 100755
> --- a/client/tests/kvm_runtest_2/kvm_install.py
> +++ b/client/tests/kvm_runtest_2/kvm_install.py
> @@ -90,7 +90,12 @@ def run_kvm_install(test, params, env):
>  	  kvm_log.info("Adding KVM_INSTALL_%s to Environment" % (k))
>            os.putenv("KVM_INSTALL_%s" % (k), str(params[k]))
>  	kvm_log.info("Running " + script + " to install kvm")
> -        os.system("cd %s; %s" % (test.bindir, script))
> +        install_result = os.system("cd %s; %s" % (test.bindir, script))
> +	if os.WEXITSTATUS(install_result) != 0:
> +          message = "Custom Script encountered an error"
> +          kvm_log.error(message)
> +          raise error.TestError, message
> +
>   

How about a helper that does os.system()  (or rather, 
commands.getstatusoutput()) and throws an exception on failure?  I 
imagine it could be used in many places.

Oh and "Custom script encountered an error" is unfriendly.  Mention the 
command line and the actual output.
Lucas Meneghel Rodrigues May 28, 2009, 6:25 a.m. UTC | #2
On Sun, 2009-05-24 at 17:48 +0300, Avi Kivity wrote:
> Mike Burns wrote:
> > Signed-off-by: Mike Burns <mburns@redhat.com>
> > ---
> >  client/tests/kvm_runtest_2/kvm_install.py |    7 ++++++-
> >  1 files changed, 6 insertions(+), 1 deletions(-)
> >
> > diff --git a/client/tests/kvm_runtest_2/kvm_install.py b/client/tests/kvm_runtest_2/kvm_install.py
> > index ebd8b7d..392ef0c 100755
> > --- a/client/tests/kvm_runtest_2/kvm_install.py
> > +++ b/client/tests/kvm_runtest_2/kvm_install.py
> > @@ -90,7 +90,12 @@ def run_kvm_install(test, params, env):
> >  	  kvm_log.info("Adding KVM_INSTALL_%s to Environment" % (k))
> >            os.putenv("KVM_INSTALL_%s" % (k), str(params[k]))
> >  	kvm_log.info("Running " + script + " to install kvm")
> > -        os.system("cd %s; %s" % (test.bindir, script))
> > +        install_result = os.system("cd %s; %s" % (test.bindir, script))
> > +	if os.WEXITSTATUS(install_result) != 0:
> > +          message = "Custom Script encountered an error"
> > +          kvm_log.error(message)
> > +          raise error.TestError, message
> > +
> >   
> 
> How about a helper that does os.system()  (or rather, 
> commands.getstatusoutput()) and throws an exception on failure?  I 
> imagine it could be used in many places.

utils.system() does that. If we have exit code != 0, it throws an
error.CmdError exception.
Avi Kivity June 1, 2009, 7:57 a.m. UTC | #3
Lucas Meneghel Rodrigues wrote:
> On Sun, 2009-05-24 at 17:48 +0300, Avi Kivity wrote:
>   
>> Mike Burns wrote:
>>     
>>> Signed-off-by: Mike Burns <mburns@redhat.com>
>>> ---
>>>  client/tests/kvm_runtest_2/kvm_install.py |    7 ++++++-
>>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/client/tests/kvm_runtest_2/kvm_install.py b/client/tests/kvm_runtest_2/kvm_install.py
>>> index ebd8b7d..392ef0c 100755
>>> --- a/client/tests/kvm_runtest_2/kvm_install.py
>>> +++ b/client/tests/kvm_runtest_2/kvm_install.py
>>> @@ -90,7 +90,12 @@ def run_kvm_install(test, params, env):
>>>  	  kvm_log.info("Adding KVM_INSTALL_%s to Environment" % (k))
>>>            os.putenv("KVM_INSTALL_%s" % (k), str(params[k]))
>>>  	kvm_log.info("Running " + script + " to install kvm")
>>> -        os.system("cd %s; %s" % (test.bindir, script))
>>> +        install_result = os.system("cd %s; %s" % (test.bindir, script))
>>> +	if os.WEXITSTATUS(install_result) != 0:
>>> +          message = "Custom Script encountered an error"
>>> +          kvm_log.error(message)
>>> +          raise error.TestError, message
>>> +
>>>   
>>>       
>> How about a helper that does os.system()  (or rather, 
>> commands.getstatusoutput()) and throws an exception on failure?  I 
>> imagine it could be used in many places.
>>     
>
> utils.system() does that. If we have exit code != 0, it throws an
> error.CmdError exception.
>   

Well let's use it then.  Every time I see 'raise' used I'm going to 
complain, so it will be a lot more efficient as well as smaller code.
Mike Burns June 1, 2009, 6:22 p.m. UTC | #4
Avi Kivity wrote:
> Lucas Meneghel Rodrigues wrote:
>> On Sun, 2009-05-24 at 17:48 +0300, Avi Kivity wrote:
>>  
>>> Mike Burns wrote:
>>>    
>>>> Signed-off-by: Mike Burns <mburns@redhat.com>
>>>> ---
>>>>  client/tests/kvm_runtest_2/kvm_install.py |    7 ++++++-
>>>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/client/tests/kvm_runtest_2/kvm_install.py
>>>> b/client/tests/kvm_runtest_2/kvm_install.py
>>>> index ebd8b7d..392ef0c 100755
>>>> --- a/client/tests/kvm_runtest_2/kvm_install.py
>>>> +++ b/client/tests/kvm_runtest_2/kvm_install.py
>>>> @@ -90,7 +90,12 @@ def run_kvm_install(test, params, env):
>>>>        kvm_log.info("Adding KVM_INSTALL_%s to Environment" % (k))
>>>>            os.putenv("KVM_INSTALL_%s" % (k), str(params[k]))
>>>>      kvm_log.info("Running " + script + " to install kvm")
>>>> -        os.system("cd %s; %s" % (test.bindir, script))
>>>> +        install_result = os.system("cd %s; %s" % (test.bindir,
>>>> script))
>>>> +    if os.WEXITSTATUS(install_result) != 0:
>>>> +          message = "Custom Script encountered an error"
>>>> +          kvm_log.error(message)
>>>> +          raise error.TestError, message
>>>> +
>>>>         
>>> How about a helper that does os.system()  (or rather,
>>> commands.getstatusoutput()) and throws an exception on failure?  I
>>> imagine it could be used in many places.
>>>     
>>
>> utils.system() does that. If we have exit code != 0, it throws an
>> error.CmdError exception.
>>   
>
> Well let's use it then.  Every time I see 'raise' used I'm going to
> complain, so it will be a lot more efficient as well as smaller code.
>
Agreed.  I'll rework this and my other patches and get them re-posted in
the next day or two.

Mike
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/client/tests/kvm_runtest_2/kvm_install.py b/client/tests/kvm_runtest_2/kvm_install.py
index ebd8b7d..392ef0c 100755
--- a/client/tests/kvm_runtest_2/kvm_install.py
+++ b/client/tests/kvm_runtest_2/kvm_install.py
@@ -90,7 +90,12 @@  def run_kvm_install(test, params, env):
 	  kvm_log.info("Adding KVM_INSTALL_%s to Environment" % (k))
           os.putenv("KVM_INSTALL_%s" % (k), str(params[k]))
 	kvm_log.info("Running " + script + " to install kvm")
-        os.system("cd %s; %s" % (test.bindir, script))
+        install_result = os.system("cd %s; %s" % (test.bindir, script))
+	if os.WEXITSTATUS(install_result) != 0:
+          message = "Custom Script encountered an error"
+          kvm_log.error(message)
+          raise error.TestError, message
+
 	kvm_log.info("Completed %s" % (script))
 
     # invalid installation mode