[PATCH] D50397: [lit, python3] Update lit error logging to work correctly in python3 and other test fixes

[Zachary Turner via llvm-commits]

We have a function somewhere in lit utils that does this, but I’m OOO so I
can’t look for it. Can you try to find it?
On Tue, Aug 7, 2018 at 1:29 PM Stella Stamenova via Phabricator <
reviews at reviews.llvm.org> wrote:

> stella.stamenova created this revision.
> stella.stamenova added reviewers: asmith, zturner.
> Herald added subscribers: llvm-commits, delcypher.
>
> In Python2 'unicode' is a distinct type from 'str', but in Python3
> 'unicode' does not exist and instead all 'str' objects are Unicode string.
> This change updates the logic in the test logging for lit to correctly
> process each of the types, and more importantly, to not just fail in
> Python3.
>
> This change also reverses the use of quotes in several of the cfg files.
> By using '""' we are guaranteeing that the resulting path will work
> correctly on Windows while "''" only works correctly sometimes. This also
> fixes one of the failing tests.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D50397
>
> Files:
>   utils/lit/lit/Test.py
>   utils/lit/lit/llvm/config.py
>   utils/lit/tests/Inputs/shtest-env/lit.cfg
>   utils/lit/tests/Inputs/shtest-shell/lit.cfg
>   utils/lit/tests/Inputs/shtest-timeout/lit.cfg
>   utils/lit/tests/lit.cfg
>   utils/lit/tests/shtest-timeout.py
>
>
> Index: utils/lit/tests/shtest-timeout.py
> ===================================================================
> --- utils/lit/tests/shtest-timeout.py
> +++ utils/lit/tests/shtest-timeout.py
> @@ -1,8 +1,5 @@
>  # REQUIRES: python-psutil
>
> -# PR33944
> -# XFAIL: windows
> -
>  # FIXME: This test is fragile because it relies on time which can
>  # be affected by system performance. In particular we are currently
>  # assuming that `short.py` can be successfully executed within 2
> Index: utils/lit/tests/lit.cfg
> ===================================================================
> --- utils/lit/tests/lit.cfg
> +++ utils/lit/tests/lit.cfg
> @@ -40,7 +40,7 @@
>              src_root, 'tests', 'Inputs')))
>  config.substitutions.append(('%{lit}', "%%{python} %s" % (
>              os.path.join(lit_path, 'lit.py'),)))
> -config.substitutions.append(('%{python}', "'%s'" % (sys.executable)))
> +config.substitutions.append(('%{python}', '"%s"' % (sys.executable)))
>
>
>  # Enable coverage.py reporting, assuming the coverage module has been
> installed
> Index: utils/lit/tests/Inputs/shtest-timeout/lit.cfg
> ===================================================================
> --- utils/lit/tests/Inputs/shtest-timeout/lit.cfg
> +++ utils/lit/tests/Inputs/shtest-timeout/lit.cfg
> @@ -29,4 +29,4 @@
>  config.target_triple = '(unused)'
>  src_root = os.path.join(config.test_source_root, '..')
>  config.environment['PYTHONPATH'] = src_root
> -config.substitutions.append(('%{python}', "'%s'" % (sys.executable)))
> +config.substitutions.append(('%{python}', '"%s"' % (sys.executable)))
> Index: utils/lit/tests/Inputs/shtest-shell/lit.cfg
> ===================================================================
> --- utils/lit/tests/Inputs/shtest-shell/lit.cfg
> +++ utils/lit/tests/Inputs/shtest-shell/lit.cfg
> @@ -4,4 +4,4 @@
>  config.test_format = lit.formats.ShTest()
>  config.test_source_root = None
>  config.test_exec_root = None
> -config.substitutions.append(('%{python}', "'%s'" % (sys.executable)))
> +config.substitutions.append(('%{python}', '"%s"' % (sys.executable)))
> Index: utils/lit/tests/Inputs/shtest-env/lit.cfg
> ===================================================================
> --- utils/lit/tests/Inputs/shtest-env/lit.cfg
> +++ utils/lit/tests/Inputs/shtest-env/lit.cfg
> @@ -6,4 +6,4 @@
>  config.test_exec_root = None
>  config.environment['FOO'] = '1'
>  config.environment['BAR'] = '2'
> -config.substitutions.append(('%{python}', "'%s'" % (sys.executable)))
> +config.substitutions.append(('%{python}', '"%s"' % (sys.executable)))
> Index: utils/lit/lit/llvm/config.py
> ===================================================================
> --- utils/lit/lit/llvm/config.py
> +++ utils/lit/lit/llvm/config.py
> @@ -299,7 +299,7 @@
>                  'count'), verbatim=True, unresolved='fatal'),
>              ToolSubst(r'\| \bnot\b', command=FindTool('not'),
> verbatim=True, unresolved='fatal')]
>
> -        self.config.substitutions.append(('%python', "'%s'" %
> (sys.executable)))
> +        self.config.substitutions.append(('%python', '"%s"' %
> (sys.executable)))
>
>          self.add_tool_substitutions(
>              tool_patterns, [self.config.llvm_tools_dir])
> Index: utils/lit/lit/Test.py
> ===================================================================
> --- utils/lit/lit/Test.py
> +++ utils/lit/lit/Test.py
> @@ -378,10 +378,15 @@
>          fil.write(testcase_xml)
>          if self.result.code.isFailure:
>              fil.write(">\n\t<failure ><![CDATA[")
> -            if type(self.result.output) == unicode:
> -                encoded_output = self.result.output.encode("utf-8",
> 'ignore')
> -            else:
> +            # In Python2, 'str' and 'unicode' are distinct types, but in
> Python3, the type 'unicode' does not exist
> +            # and instead 'bytes' is distinct
> +            # in Python3, there's no unicode
> +            if isinstance(self.result.output, str):
>                  encoded_output = self.result.output
> +            elif isinstance(self.result.output, bytes):
> +                encoded_output = self.result.output.decode("utf-8",
> 'ignore')
> +            else:
> +                encoded_output = self.result.output.encode("utf-8",
> 'ignore')
>              # In the unlikely case that the output contains the CDATA
> terminator
>              # we wrap it by creating a new CDATA block
>              fil.write(encoded_output.replace("]]>", "]]]]><![CDATA[>"))
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180807/188b1abc/attachment.html>