[3/5] framework: handle UnicodeDecodeError

Submitted by Marek Olšák on May 2, 2018, 8:32 p.m.

Details

Message ID 20180502203247.11284-3-maraeo@gmail.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Piglit

Not browsing as part of any series.

Commit Message

Marek Olšák May 2, 2018, 8:32 p.m.
From: Marek Olšák <marek.olsak@amd.com>

This happens due to LLVM printing colored text into stdout/stderr on error.
---
 framework/test/base.py | 3 +++
 1 file changed, 3 insertions(+)

Patch hide | download patch | download mbox

diff --git a/framework/test/base.py b/framework/test/base.py
index 134b87245..f187c0210 100644
--- a/framework/test/base.py
+++ b/framework/test/base.py
@@ -369,20 +369,23 @@  class Test(object):
                 os.killpg(os.getpgid(proc.pid), signal.SIGKILL)
 
             # Since the process isn't running it's safe to get any remaining
             # stdout/stderr values out and store them.
             self.result.out, self.result.err = proc.communicate()
 
             raise TestRunError(
                 'Test run time exceeded timeout value ({} seconds)\n'.format(
                     self.timeout),
                 'timeout')
+        # LLVM prints colored text into stdout/stderr on error, which raises:
+        except UnicodeDecodeError as e:
+            raise TestRunError("UnicodeDecodeError.\n", 'crash')
 
         # The setter handles the bytes/unicode conversion
         self.result.out = out
         self.result.err = err
         self.result.returncode = returncode
 
     def __eq__(self, other):
         return self.command == other.command
 
     def __ne__(self, other):

Comments

Quoting Marek Olšák (2018-05-02 13:32:45)
> From: Marek Olšák <marek.olsak@amd.com>
> 
> This happens due to LLVM printing colored text into stdout/stderr on error.
> ---
>  framework/test/base.py | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/framework/test/base.py b/framework/test/base.py
> index 134b87245..f187c0210 100644
> --- a/framework/test/base.py
> +++ b/framework/test/base.py
> @@ -369,20 +369,23 @@ class Test(object):
>                  os.killpg(os.getpgid(proc.pid), signal.SIGKILL)
>  
>              # Since the process isn't running it's safe to get any remaining
>              # stdout/stderr values out and store them.
>              self.result.out, self.result.err = proc.communicate()
>  
>              raise TestRunError(
>                  'Test run time exceeded timeout value ({} seconds)\n'.format(
>                      self.timeout),
>                  'timeout')
> +        # LLVM prints colored text into stdout/stderr on error, which raises:
> +        except UnicodeDecodeError as e:
> +            raise TestRunError("UnicodeDecodeError.\n", 'crash')

This seems odd to me, Popen.communicate() returns bytes, and the conversion is
done right after this (it's hideous, but there is a setter function for
result.out and result.err that converts bytes to unicode), but is uses replace
for characters it doesn't understand.

>  
>          # The setter handles the bytes/unicode conversion
>          self.result.out = out
>          self.result.err = err
>          self.result.returncode = returncode
>  
>      def __eq__(self, other):
>          return self.command == other.command
>  
>      def __ne__(self, other):
> -- 
> 2.17.0
> 
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
On Wed, May 2, 2018 at 4:48 PM, Dylan Baker <dylan@pnwbakers.com> wrote:

> Quoting Marek Olšák (2018-05-02 13:32:45)
> > From: Marek Olšák <marek.olsak@amd.com>
> >
> > This happens due to LLVM printing colored text into stdout/stderr on
> error.
> > ---
> >  framework/test/base.py | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/framework/test/base.py b/framework/test/base.py
> > index 134b87245..f187c0210 100644
> > --- a/framework/test/base.py
> > +++ b/framework/test/base.py
> > @@ -369,20 +369,23 @@ class Test(object):
> >                  os.killpg(os.getpgid(proc.pid), signal.SIGKILL)
> >
> >              # Since the process isn't running it's safe to get any
> remaining
> >              # stdout/stderr values out and store them.
> >              self.result.out, self.result.err = proc.communicate()
> >
> >              raise TestRunError(
> >                  'Test run time exceeded timeout value ({}
> seconds)\n'.format(
> >                      self.timeout),
> >                  'timeout')
> > +        # LLVM prints colored text into stdout/stderr on error, which
> raises:
> > +        except UnicodeDecodeError as e:
> > +            raise TestRunError("UnicodeDecodeError.\n", 'crash')
>
> This seems odd to me, Popen.communicate() returns bytes, and the
> conversion is
> done right after this (it's hideous, but there is a setter function for
> result.out and result.err that converts bytes to unicode), but is uses
> replace
> for characters it doesn't understand.
>

I doubt that it can filter out shell colors, though I'm not sure right now.
If I encounter the error again, can I just push this without Rb?

Marek
Quoting Marek Olšák (2018-05-02 13:55:06)
> On Wed, May 2, 2018 at 4:48 PM, Dylan Baker <dylan@pnwbakers.com> wrote:
> 
>     Quoting Marek Olšák (2018-05-02 13:32:45)
>     > From: Marek Olšák <marek.olsak@amd.com>
>     >
>     > This happens due to LLVM printing colored text into stdout/stderr on
>     error.
>     > ---
>     >  framework/test/base.py | 3 +++
>     >  1 file changed, 3 insertions(+)
>     >
>     > diff --git a/framework/test/base.py b/framework/test/base.py
>     > index 134b87245..f187c0210 100644
>     > --- a/framework/test/base.py
>     > +++ b/framework/test/base.py
>     > @@ -369,20 +369,23 @@ class Test(object):
>     >                  os.killpg(os.getpgid(proc.pid), signal.SIGKILL)
>     > 
>     >              # Since the process isn't running it's safe to get any
>     remaining
>     >              # stdout/stderr values out and store them.
>     >              self.result.out, self.result.err = proc.communicate()
>     > 
>     >              raise TestRunError(
>     >                  'Test run time exceeded timeout value ({} seconds)\
>     n'.format(
>     >                      self.timeout),
>     >                  'timeout')
>     > +        # LLVM prints colored text into stdout/stderr on error, which
>     raises:
>     > +        except UnicodeDecodeError as e:
>     > +            raise TestRunError("UnicodeDecodeError.\n", 'crash')
> 
>     This seems odd to me, Popen.communicate() returns bytes, and the conversion
>     is
>     done right after this (it's hideous, but there is a setter function for
>     result.out and result.err that converts bytes to unicode), but is uses
>     replace
>     for characters it doesn't understand.
> 
> 
> I doubt that it can filter out shell colors, though I'm not sure right now. If
> I encounter the error again, can I just push this without Rb?
> 
> Marek

Actually, I take that back, apparently univeral_newlines in python3 makes out
and err a text stream instead of a byte stream. I think in the long term we
should dump universal newlines to avoid this sort of problem, but for now this
seems reasonable.

Reviewed-by: Dylan Baker <dylan@pnwbakers.com>