[PATCH XTF 0/3] Runner exit code clean up

[PATCH XTF 0/3] Runner exit code clean up

[Wei Liu]

Wei Liu (3):
  xtf-runner: sync all test states
  xtf-runner: provide a set of exit codes for different states
  xtf-runner: regularise runner exit code

 xtf-runner | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH XTF 1/3] xtf-runner: sync all test states

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xtf-runner | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xtf-runner b/xtf-runner
index 50a5e96..ad7dcf9 100755
--- a/xtf-runner
+++ b/xtf-runner
@@ -17,6 +17,9 @@ try:
 except ImportError:
     import simplejson as json
 
+# All states of a test, keep in sync with C code report.h.
+# Note that warning is not a state on its own.
+all_states = [ 'SUCCESS', 'SKIP', 'ERROR', 'FAILURE' ]
 
 # All test categories and configurations
 all_categories   = ("special", "functional", "xsa", "utility")
@@ -161,7 +164,7 @@ def run_test(test):
     if not "Test result:" in test_result:
         return "ERROR"
 
-    for res in ("SUCCESS", "SKIP", "FAILURE"):
+    for res in all_states:
 
         if res in test_result:
             return res
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH XTF 1/3] xtf-runner: sync all test states

[Andrew Cooper]

On 21/07/2016 16:44, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  xtf-runner | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/xtf-runner b/xtf-runner
> index 50a5e96..ad7dcf9 100755
> --- a/xtf-runner
> +++ b/xtf-runner
> @@ -17,6 +17,9 @@ try:
>  except ImportError:
>      import simplejson as json
>  
> +# All states of a test, keep in sync with C code report.h.
> +# Note that warning is not a state on its own.
> +all_states = [ 'SUCCESS', 'SKIP', 'ERROR', 'FAILURE' ]

This is never going to change, so using a tuple (like all_categories) is
more efficient.

Also, I would recommend all_results as a sightly more appropriate name.

Finally, if you aren't already aware, please check the result of `make
pylint` (not that I have spotted specific issues).

~Andrew

>  
>  # All test categories and configurations
>  all_categories   = ("special", "functional", "xsa", "utility")
> @@ -161,7 +164,7 @@ def run_test(test):
>      if not "Test result:" in test_result:
>          return "ERROR"
>  
> -    for res in ("SUCCESS", "SKIP", "FAILURE"):
> +    for res in all_states:
>  
>          if res in test_result:
>              return res


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH XTF 2/3] xtf-runner: provide a set of exit codes for different states

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xtf-runner | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/xtf-runner b/xtf-runner
index ad7dcf9..17ce933 100755
--- a/xtf-runner
+++ b/xtf-runner
@@ -21,6 +21,14 @@ except ImportError:
 # Note that warning is not a state on its own.
 all_states = [ 'SUCCESS', 'SKIP', 'ERROR', 'FAILURE' ]
 
+# Return the exit code for different states.
+# Avoid using 1 and 2 because python interpreter uses them.
+def exit_code(state):
+    if state == 'SUCCESS': return 0
+    if state == 'SKIP':    return 3
+    if state == 'ERROR':   return 4
+    if state == 'FAILURE': return 5
+
 # All test categories and configurations
 all_categories   = ("special", "functional", "xsa", "utility")
 pv_environments  = ("pv64", "pv32pae")
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH XTF 2/3] xtf-runner: provide a set of exit codes for different states

[Ian Jackson]

Wei Liu writes ("[PATCH XTF 2/3] xtf-runner: provide a set of exit codes for different states"):
> +# Return the exit code for different states.
> +# Avoid using 1 and 2 because python interpreter uses them.

Python generates `2' too ?  That's quite exciting.  When ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH XTF 2/3] xtf-runner: provide a set of exit codes for different states

[Wei Liu]

On Thu, Jul 21, 2016 at 05:27:24PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH XTF 2/3] xtf-runner: provide a set of exit codes for different states"):
> > +# Return the exit code for different states.
> > +# Avoid using 1 and 2 because python interpreter uses them.
> 
> Python generates `2' too ?  That's quite exciting.  When ?
> 

There is vague description on this:

sys.exit([arg])
...
Unix programs generally use 2 for command line syntax errors and 1 for
all other kind of errors. If another type of object is passed, None is
...

It doesn't explicitly say the interpreter uses 2. But we'd better err on
the safe side.

Wei.

> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH XTF 2/3] xtf-runner: provide a set of exit codes for different states

[Wei Liu]

On Thu, Jul 21, 2016 at 05:29:48PM +0100, Wei Liu wrote:
> On Thu, Jul 21, 2016 at 05:27:24PM +0100, Ian Jackson wrote:
> > Wei Liu writes ("[PATCH XTF 2/3] xtf-runner: provide a set of exit codes for different states"):
> > > +# Return the exit code for different states.
> > > +# Avoid using 1 and 2 because python interpreter uses them.
> > 
> > Python generates `2' too ?  That's quite exciting.  When ?
> > 
> 
> There is vague description on this:
> 
> sys.exit([arg])
> ...
> Unix programs generally use 2 for command line syntax errors and 1 for
> all other kind of errors. If another type of object is passed, None is
> ...
> 
> It doesn't explicitly say the interpreter uses 2. But we'd better err on
> the safe side.
> 

And in practice:

$ python --xxx && echo $?
2

Indeed it uses 2.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH XTF 2/3] xtf-runner: provide a set of exit codes for different states

[Andrew Cooper]

On 21/07/2016 16:44, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  xtf-runner | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/xtf-runner b/xtf-runner
> index ad7dcf9..17ce933 100755
> --- a/xtf-runner
> +++ b/xtf-runner
> @@ -21,6 +21,14 @@ except ImportError:
>  # Note that warning is not a state on its own.
>  all_states = [ 'SUCCESS', 'SKIP', 'ERROR', 'FAILURE' ]
>  
> +# Return the exit code for different states.
> +# Avoid using 1 and 2 because python interpreter uses them.
> +def exit_code(state):
> +    if state == 'SUCCESS': return 0
> +    if state == 'SKIP':    return 3
> +    if state == 'ERROR':   return 4
> +    if state == 'FAILURE': return 5

You can make a pseudo switch statement with a dictionary by doing:

def exit_code(state):
    """ Convert a test result to an xtf-runner exit code. """
    return { "SUCCESS": 0,
             "SKIP":    3,
             "ERROR":   4,
             "FAILURE": 5,
    }.get(state, 4)

This also causes the default return value in the case that state isn't
matched to be ERROR, rather than None which translates to 0 when the
process exists.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH XTF 3/3] xtf-runner: regularise runner exit code

[Wei Liu]

Report the first "ERROR" and "FAILURE" if found, otherwise report "SKIP"
if found. Eventually if everything is ok the exit code will be 0.

See runner code for numeric exit code space.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xtf-runner | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/xtf-runner b/xtf-runner
index 17ce933..ebe5c27 100755
--- a/xtf-runner
+++ b/xtf-runner
@@ -249,17 +249,23 @@ def run_tests(args):
     if not len(tests):
         raise RunnerError("No tests to run")
 
-    rc = 0
+    rc = exit_code('SUCCESS')
     results = []
 
     for test in tests:
 
         res = run_test(test)
-        if res != "SUCCESS":
-            rc = 1
+        if res in ("ERROR", "FAILURE") and rc == exit_code('SUCCESS'):
+            rc = exit_code(res)
 
         results.append(res)
 
+    if rc == exit_code('SUCCESS'):
+        for res in results:
+            if res == 'SKIP':
+                rc = exit_code('SKIP')
+                break
+
     print "\nCombined test results:"
 
     for test, res in zip(tests, results):
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH XTF 3/3] xtf-runner: regularise runner exit code

[Andrew Cooper]

On 21/07/2016 16:44, Wei Liu wrote:
> Report the first "ERROR" and "FAILURE" if found, otherwise report "SKIP"
> if found. Eventually if everything is ok the exit code will be 0.
>
> See runner code for numeric exit code space.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  xtf-runner | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/xtf-runner b/xtf-runner
> index 17ce933..ebe5c27 100755
> --- a/xtf-runner
> +++ b/xtf-runner
> @@ -249,17 +249,23 @@ def run_tests(args):
>      if not len(tests):
>          raise RunnerError("No tests to run")
>  
> -    rc = 0
> +    rc = exit_code('SUCCESS')

This logic would be easier to express if you use the indices of
all_results as a measure of severity.

e.g.

rc = all_results.index('SUCCESS')

>      results = []
>  
>      for test in tests:
>  
>          res = run_test(test)
> -        if res != "SUCCESS":
> -            rc = 1
> +        if res in ("ERROR", "FAILURE") and rc == exit_code('SUCCESS'):
> +            rc = exit_code(res)

res_idx = all_results.index(res)
if res_idx > rc:
    rc = res_idx

>  
>          results.append(res)
>  
> +    if rc == exit_code('SUCCESS'):
> +        for res in results:
> +            if res == 'SKIP':
> +                rc = exit_code('SKIP')
> +                break
> +
>      print "\nCombined test results:"
>  
>      for test, res in zip(tests, results):

And down at the end here:

return exit_code(all_results[rc])

Finally, please update the epilog text in main() to enumerate the exit
codes.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH XTF 3/3] xtf-runner: regularise runner exit code

[Wei Liu]

On Thu, Jul 21, 2016 at 08:04:59PM +0100, Andrew Cooper wrote:
> On 21/07/2016 16:44, Wei Liu wrote:
> > Report the first "ERROR" and "FAILURE" if found, otherwise report "SKIP"
> > if found. Eventually if everything is ok the exit code will be 0.
> >
> > See runner code for numeric exit code space.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  xtf-runner | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/xtf-runner b/xtf-runner
> > index 17ce933..ebe5c27 100755
> > --- a/xtf-runner
> > +++ b/xtf-runner
> > @@ -249,17 +249,23 @@ def run_tests(args):
> >      if not len(tests):
> >          raise RunnerError("No tests to run")
> >  
> > -    rc = 0
> > +    rc = exit_code('SUCCESS')
> 
> This logic would be easier to express if you use the indices of
> all_results as a measure of severity.
> 
> e.g.
> 
> rc = all_results.index('SUCCESS')
> 
> >      results = []
> >  
> >      for test in tests:
> >  
> >          res = run_test(test)
> > -        if res != "SUCCESS":
> > -            rc = 1
> > +        if res in ("ERROR", "FAILURE") and rc == exit_code('SUCCESS'):
> > +            rc = exit_code(res)
> 
> res_idx = all_results.index(res)
> if res_idx > rc:
>     rc = res_idx

I intended to report the first "error" or "failure" encountered.

This would cause a FAILURE to overwrite previous ERROR result. Is that
what you want? 

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH XTF 3/3] xtf-runner: regularise runner exit code

[Andrew Cooper]

On 22/07/16 10:29, Wei Liu wrote:

> On Thu, Jul 21, 2016 at 08:04:59PM +0100, Andrew Cooper wrote:
>> On 21/07/2016 16:44, Wei Liu wrote:
>>> Report the first "ERROR" and "FAILURE" if found, otherwise report "SKIP"
>>> if found. Eventually if everything is ok the exit code will be 0.
>>>
>>> See runner code for numeric exit code space.
>>>
>>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>>> ---
>>>   xtf-runner | 12 +++++++++---
>>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/xtf-runner b/xtf-runner
>>> index 17ce933..ebe5c27 100755
>>> --- a/xtf-runner
>>> +++ b/xtf-runner
>>> @@ -249,17 +249,23 @@ def run_tests(args):
>>>       if not len(tests):
>>>           raise RunnerError("No tests to run")
>>>   
>>> -    rc = 0
>>> +    rc = exit_code('SUCCESS')
>> This logic would be easier to express if you use the indices of
>> all_results as a measure of severity.
>>
>> e.g.
>>
>> rc = all_results.index('SUCCESS')
>>
>>>       results = []
>>>   
>>>       for test in tests:
>>>   
>>>           res = run_test(test)
>>> -        if res != "SUCCESS":
>>> -            rc = 1
>>> +        if res in ("ERROR", "FAILURE") and rc == exit_code('SUCCESS'):
>>> +            rc = exit_code(res)
>> res_idx = all_results.index(res)
>> if res_idx > rc:
>>      rc = res_idx
> I intended to report the first "error" or "failure" encountered.
>
> This would cause a FAILURE to overwrite previous ERROR result. Is that
> what you want?

When running more than one test, the overall result should be the most 
severe.  So yes, a subsequent FAILURE should override an ERROR.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH XTF 3/3] xtf-runner: regularise runner exit code

[Ian Jackson]

Andrew Cooper writes ("Re: [PATCH XTF 3/3] xtf-runner: regularise runner exit code"):
> On 22/07/16 10:29, Wei Liu wrote:
> > This would cause a FAILURE to overwrite previous ERROR result. Is that
> > what you want?
> 
> When running more than one test, the overall result should be the most 
> severe.  So yes, a subsequent FAILURE should override an ERROR.

"Error" is surely a more severe problem than "Failure".
In particular, Error might mask a failure.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH XTF 3/3] xtf-runner: regularise runner exit code

[Andrew Cooper]

On 25/07/16 12:25, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [PATCH XTF 3/3] xtf-runner: regularise runner exit code"):
>> On 22/07/16 10:29, Wei Liu wrote:
>>> This would cause a FAILURE to overwrite previous ERROR result. Is that
>>> what you want?
>> When running more than one test, the overall result should be the most 
>> severe.  So yes, a subsequent FAILURE should override an ERROR.
> "Error" is surely a more severe problem than "Failure".

No.  Error means "something unexpected went wrong", while Failure is
"the test worked, and identified that the area under test is defective".

Error is typically a test case bug or infrastructure issue.

> In particular, Error might mask a failure.

Indeed it could.  Either should cause a prompt investigation and effort
to resolve the issues, but one must be more severe than the other and
this is the way things are specified.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH XTF 3/3] xtf-runner: regularise runner exit code

[Ian Jackson]

Andrew Cooper writes ("Re: [PATCH XTF 3/3] xtf-runner: regularise runner exit code"):
> On 25/07/16 12:25, Ian Jackson wrote:
> > In particular, Error might mask a failure.
> 
> Indeed it could.  Either should cause a prompt investigation and effort
> to resolve the issues, but one must be more severe than the other and
> this is the way things are specified.

Is this really how XenRT expects things to be explained to it ?

This seems to be a very unusual approach.  Normally, in any program,
the unbounded class of errors will be always reported in the exit
status.

If we are running xtf inside osstest, I trust we aren't relying on
this aggregated exit status ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH XTF 3/3] xtf-runner: regularise runner exit code

[Wei Liu]

on Mon, Jul 25, 2016 at 06:05:21PM +0100, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [PATCH XTF 3/3] xtf-runner: regularise runner exit code"):
> > On 25/07/16 12:25, Ian Jackson wrote:
> > > In particular, Error might mask a failure.
> > 
> > Indeed it could.  Either should cause a prompt investigation and effort
> > to resolve the issues, but one must be more severe than the other and
> > this is the way things are specified.
> 
> Is this really how XenRT expects things to be explained to it ?
> 
> This seems to be a very unusual approach.  Normally, in any program,
> the unbounded class of errors will be always reported in the exit
> status.
> 
> If we are running xtf inside osstest, I trust we aren't relying on
> this aggregated exit status ?
> 

No, we don't rely on aggregated exit status.

Wei.

> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel