[Piglit] [PATCH] exectest.py: Return early if _run_command() hits the except block

[Jose Fonseca]

Yes, that looks sensible and I've confirmed it fixed the mis-categorized skip->crashes too.

Thanks Dylan.

Reviewed-by: Jose Fonseca <jfonseca at vmware.com>

Jose

----- Original Message -----
> If the except block is hit in Test._run_command(), the test has a status
> of skip, but the test continues to run anyway. This is not consistent
> with the way Test.check_for_skip_scenario() works, and means more time
> is spent in python than needs to be.
> 
> This patch adds a small check after _run_command returns, if
> self.results['result'] has been set to skip then Test.run() returns
> immediately.
> 
> Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
> 
> Jose, here's a different aproach to the problem, and I think this one is
> better because it is more consistent with the way the python framework
> handles skips, and it saves time on what would be wasted processing.
> ---
>  framework/exectest.py            |  7 +++++++
>  framework/tests/exectest_test.py | 27 +++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/framework/exectest.py b/framework/exectest.py
> index a833066..e55274e 100644
> --- a/framework/exectest.py
> +++ b/framework/exectest.py
> @@ -152,6 +152,12 @@ class Test(object):
>              if "Got spurious window resize" not in self.result['out']:
>                  break
>  
> +        # If the result is skip then the test wasn't run, return early
> +        # This usually is triggered when a test is not built for a specific
> +        # platform
> +        if self.result['result'] == 'skip':
> +            return
> +
>          self.result['result'] = 'fail'
>          self.interpret_result()
>  
> @@ -202,6 +208,7 @@ class Test(object):
>              # Piglit should not report that test as having
>              # failed.
>              if e.errno == errno.ENOENT:
> +                self.result['result'] = 'skip'
>                  out = ("PIGLIT: {'result': 'skip'}\n"
>                         "Test executable not found.\n")
>                  err = ""
> diff --git a/framework/tests/exectest_test.py
> b/framework/tests/exectest_test.py
> index 2f0569f..5a81ec0 100644
> --- a/framework/tests/exectest_test.py
> +++ b/framework/tests/exectest_test.py
> @@ -23,6 +23,21 @@
>  from framework.exectest import PiglitTest, Test
>  
>  
> +# Helpers
> +class TestTest(Test):
> +    """ A class for testing that implements a dummy interpret_result
> +
> +    interpret_result() can ve overwritten by setting the
> +    self.test_interpret_result name
> +
> +    """
> +    test_interpret_result = lambda: None
> +
> +    def interpret_result(self):
> +        self.test_interpret_result()
> +
> +
> +# Tests
>  def test_initialize_test():
>      """ Test initializes """
>      Test('/bin/true')
> @@ -31,3 +46,15 @@ def test_initialize_test():
>  def test_initialize_piglittest():
>      """ Test that PiglitTest initializes correctly """
>      PiglitTest('/bin/true')
> +
> +
> +def test_run_return_early():
> +    """ Test.run() exits early when Test._run_command() has exception """
> +    def helper():
> +        raise AssertionError("The test didn't return early")
> +
> +    # Of course, this won't work if you actually have a foobarcommand in
> your
> +    # path...
> +    test = TestTest(['foobarcommand'])
> +    test.test_interpret_result = helper
> +    test.run()
> --
> 2.0.0
> 
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/piglit&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=NMr9uy2iTjWVixC0wOcYCWEIYhfo80qKwRgdodpoDzA%3D%0A&m=UJt%2BNuSfEgSuD88bnOIhjB0V3eQcHe%2FJbFJ5IlCexZk%3D%0A&s=43067aaa051bc8ad0278e6aff8775c056f2eca195c660e84a7ef6bae7858c890
>