[1/2] framework: Write exceptions and traceback to /dev/stderr and json results.

Submitted by Jose Fonseca on Dec. 7, 2015, 2:25 p.m.

Details

Message ID 1449498360-13792-1-git-send-email-jfonseca@vmware.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Piglit

Not browsing as part of any series.

Commit Message

Jose Fonseca Dec. 7, 2015, 2:25 p.m.
Both were being completely hidden.

Junit is probably still busted.  But the right fix is beyond any doubt
to not catch generic Python exceptions, or override stderr, at all.
---
 framework/results.py       | 3 ++-
 framework/test/base.py     | 9 ++++++---
 templates/test_result.mako | 8 ++++++++
 3 files changed, 16 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/framework/results.py b/framework/results.py
index eeffcb7..ef19fd4 100644
--- a/framework/results.py
+++ b/framework/results.py
@@ -196,6 +196,7 @@  class TestResult(object):
             'subtests': self.subtests,
             'time': self.time,
             'exception': self.exception,
+            'traceback': self.traceback,
             'dmesg': self.dmesg,
         }
         return obj
@@ -215,7 +216,7 @@  class TestResult(object):
         # pylint: disable=assigning-non-slot
         inst = cls()
 
-        for each in ['returncode', 'command', 'exception', 'environment',
+        for each in ['returncode', 'command', 'exception', 'traceback', 'environment',
                      'time', 'result', 'dmesg']:
             if each in dict_:
                 setattr(inst, each, dict_[each])
diff --git a/framework/test/base.py b/framework/test/base.py
index bf998d8..6cff2d3 100644
--- a/framework/test/base.py
+++ b/framework/test/base.py
@@ -184,11 +184,14 @@  class Test(object):
             # This is a rare case where a bare exception is okay, since we're
             # using it to log exceptions
             except:
-                exception = sys.exc_info()
+                exc_type, exc_value, exc_traceback = sys.exc_info()
+                if sys.platform == 'linux2':
+                    stderr = open("/dev/stderr", "wt")
+                    traceback.print_exc(file=stderr)
                 self.result.result = 'fail'
-                self.result.exception = "{}{}".format(*exception[:2])
+                self.result.exception = "{}{}".format(exc_type, exc_value)
                 self.result.traceback = "".join(
-                    traceback.format_tb(exception[2]))
+                    traceback.format_tb(exc_traceback))
 
             log.log(self.result.result)
         else:
diff --git a/templates/test_result.mako b/templates/test_result.mako
index 229a5a7..ff08797 100644
--- a/templates/test_result.mako
+++ b/templates/test_result.mako
@@ -75,6 +75,14 @@ 
           </pre>${value.command}</pre>
         </td>
       </tr>
+    % if value.exception:
+      <tr>
+        <td>Exception</td>
+        <td>
+          <pre>${value.exception | h}</pre>
+        </td>
+      </tr>
+    % endif
     % if value.traceback:
       <tr>
         <td>Traceback</td>

Comments

On Mon, Dec 07, 2015 at 02:25:59PM +0000, Jose Fonseca wrote:
> Both were being completely hidden.
> 
> Junit is probably still busted.  But the right fix is beyond any doubt
> to not catch generic Python exceptions, or override stderr, at all.
> ---
>  framework/results.py       | 3 ++-
>  framework/test/base.py     | 9 ++++++---
>  templates/test_result.mako | 8 ++++++++
>  3 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/framework/results.py b/framework/results.py
> index eeffcb7..ef19fd4 100644
> --- a/framework/results.py
> +++ b/framework/results.py
> @@ -196,6 +196,7 @@ class TestResult(object):
>              'subtests': self.subtests,
>              'time': self.time,
>              'exception': self.exception,
> +            'traceback': self.traceback,
>              'dmesg': self.dmesg,
>          }
>          return obj
> @@ -215,7 +216,7 @@ class TestResult(object):
>          # pylint: disable=assigning-non-slot
>          inst = cls()
>  
> -        for each in ['returncode', 'command', 'exception', 'environment',
> +        for each in ['returncode', 'command', 'exception', 'traceback', 'environment',
>                       'time', 'result', 'dmesg']:
>              if each in dict_:
>                  setattr(inst, each, dict_[each])
> diff --git a/framework/test/base.py b/framework/test/base.py
> index bf998d8..6cff2d3 100644
> --- a/framework/test/base.py
> +++ b/framework/test/base.py
> @@ -184,11 +184,14 @@ class Test(object):
>              # This is a rare case where a bare exception is okay, since we're
>              # using it to log exceptions
>              except:
> -                exception = sys.exc_info()
> +                exc_type, exc_value, exc_traceback = sys.exc_info()
> +                if sys.platform == 'linux2':
> +                    stderr = open("/dev/stderr", "wt")
> +                    traceback.print_exc(file=stderr)

does "traceback.print_exc(file=sys.stderr)" not work?

>                  self.result.result = 'fail'
> -                self.result.exception = "{}{}".format(*exception[:2])
> +                self.result.exception = "{}{}".format(exc_type, exc_value)
>                  self.result.traceback = "".join(
> -                    traceback.format_tb(exception[2]))
> +                    traceback.format_tb(exc_traceback))
>  
>              log.log(self.result.result)
>          else:
> diff --git a/templates/test_result.mako b/templates/test_result.mako
> index 229a5a7..ff08797 100644
> --- a/templates/test_result.mako
> +++ b/templates/test_result.mako
> @@ -75,6 +75,14 @@
>            </pre>${value.command}</pre>
>          </td>
>        </tr>
> +    % if value.exception:
> +      <tr>
> +        <td>Exception</td>
> +        <td>
> +          <pre>${value.exception | h}</pre>
> +        </td>
> +      </tr>
> +    % endif
>      % if value.traceback:
>        <tr>
>          <td>Traceback</td>
> -- 
> 2.5.0
> 
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
On 07/12/15 18:06, Dylan Baker wrote:
> On Mon, Dec 07, 2015 at 02:25:59PM +0000, Jose Fonseca wrote:
>> Both were being completely hidden.
>>
>> Junit is probably still busted.  But the right fix is beyond any doubt
>> to not catch generic Python exceptions, or override stderr, at all.
>> ---
>>   framework/results.py       | 3 ++-
>>   framework/test/base.py     | 9 ++++++---
>>   templates/test_result.mako | 8 ++++++++
>>   3 files changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/framework/results.py b/framework/results.py
>> index eeffcb7..ef19fd4 100644
>> --- a/framework/results.py
>> +++ b/framework/results.py
>> @@ -196,6 +196,7 @@ class TestResult(object):
>>               'subtests': self.subtests,
>>               'time': self.time,
>>               'exception': self.exception,
>> +            'traceback': self.traceback,
>>               'dmesg': self.dmesg,
>>           }
>>           return obj
>> @@ -215,7 +216,7 @@ class TestResult(object):
>>           # pylint: disable=assigning-non-slot
>>           inst = cls()
>>
>> -        for each in ['returncode', 'command', 'exception', 'environment',
>> +        for each in ['returncode', 'command', 'exception', 'traceback', 'environment',
>>                        'time', 'result', 'dmesg']:
>>               if each in dict_:
>>                   setattr(inst, each, dict_[each])
>> diff --git a/framework/test/base.py b/framework/test/base.py
>> index bf998d8..6cff2d3 100644
>> --- a/framework/test/base.py
>> +++ b/framework/test/base.py
>> @@ -184,11 +184,14 @@ class Test(object):
>>               # This is a rare case where a bare exception is okay, since we're
>>               # using it to log exceptions
>>               except:
>> -                exception = sys.exc_info()
>> +                exc_type, exc_value, exc_traceback = sys.exc_info()
>> +                if sys.platform == 'linux2':
>> +                    stderr = open("/dev/stderr", "wt")
>> +                    traceback.print_exc(file=stderr)
>
> does "traceback.print_exc(file=sys.stderr)" not work?

No. Not until my "Never redirect sys.stderr to /dev/null." patch I sent 
afterwards

Jose

>>                   self.result.result = 'fail'
>> -                self.result.exception = "{}{}".format(*exception[:2])
>> +                self.result.exception = "{}{}".format(exc_type, exc_value)
>>                   self.result.traceback = "".join(
>> -                    traceback.format_tb(exception[2]))
>> +                    traceback.format_tb(exc_traceback))
>>
>>               log.log(self.result.result)
>>           else:
>> diff --git a/templates/test_result.mako b/templates/test_result.mako
>> index 229a5a7..ff08797 100644
>> --- a/templates/test_result.mako
>> +++ b/templates/test_result.mako
>> @@ -75,6 +75,14 @@
>>             </pre>${value.command}</pre>
>>           </td>
>>         </tr>
>> +    % if value.exception:
>> +      <tr>
>> +        <td>Exception</td>
>> +        <td>
>> +          <pre>${value.exception | h}</pre>
>> +        </td>
>> +      </tr>
>> +    % endif
>>       % if value.traceback:
>>         <tr>
>>           <td>Traceback</td>
>> --
>> 2.5.0
>>
>> _______________________________________________
>> Piglit mailing list
>> Piglit@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/piglit
On 07/12/15 22:21, Jose Fonseca wrote:
> On 07/12/15 18:06, Dylan Baker wrote:
>> On Mon, Dec 07, 2015 at 02:25:59PM +0000, Jose Fonseca wrote:
>>> Both were being completely hidden.
>>>
>>> Junit is probably still busted.  But the right fix is beyond any doubt
>>> to not catch generic Python exceptions, or override stderr, at all.
>>> ---
>>>   framework/results.py       | 3 ++-
>>>   framework/test/base.py     | 9 ++++++---
>>>   templates/test_result.mako | 8 ++++++++
>>>   3 files changed, 16 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/framework/results.py b/framework/results.py
>>> index eeffcb7..ef19fd4 100644
>>> --- a/framework/results.py
>>> +++ b/framework/results.py
>>> @@ -196,6 +196,7 @@ class TestResult(object):
>>>               'subtests': self.subtests,
>>>               'time': self.time,
>>>               'exception': self.exception,
>>> +            'traceback': self.traceback,
>>>               'dmesg': self.dmesg,
>>>           }
>>>           return obj
>>> @@ -215,7 +216,7 @@ class TestResult(object):
>>>           # pylint: disable=assigning-non-slot
>>>           inst = cls()
>>>
>>> -        for each in ['returncode', 'command', 'exception',
>>> 'environment',
>>> +        for each in ['returncode', 'command', 'exception',
>>> 'traceback', 'environment',
>>>                        'time', 'result', 'dmesg']:
>>>               if each in dict_:
>>>                   setattr(inst, each, dict_[each])
>>> diff --git a/framework/test/base.py b/framework/test/base.py
>>> index bf998d8..6cff2d3 100644
>>> --- a/framework/test/base.py
>>> +++ b/framework/test/base.py
>>> @@ -184,11 +184,14 @@ class Test(object):
>>>               # This is a rare case where a bare exception is okay,
>>> since we're
>>>               # using it to log exceptions
>>>               except:
>>> -                exception = sys.exc_info()
>>> +                exc_type, exc_value, exc_traceback = sys.exc_info()
>>> +                if sys.platform == 'linux2':
>>> +                    stderr = open("/dev/stderr", "wt")
>>> +                    traceback.print_exc(file=stderr)
>>
>> does "traceback.print_exc(file=sys.stderr)" not work?
>
> No. Not until my "Never redirect sys.stderr to /dev/null." patch I sent
> afterwards
>
> Jose

I'd like to get my patches committed.

We can tweak things to our hearts content afterwards, but these patches 
fix real issues for me, and anybody else thats needs to debug the issues 
with piglit Python framework.  Particularly the patches to not override 
stderr and show exceptions on stderr.

For the sake of clarity, I've reordered and cleaned them all up again 
and reposted them.

If there are concrete requests/objections to the patches let me know. 
Otherwise a Reviewed/Acked-by would be appreciated.

Jose