[libcxx-commits] [PATCH] D120623: [lit] Read command stdout/stderr as text on Windows

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added a comment.

In D120623#3352655 <https://reviews.llvm.org/D120623#3352655>, @rnk wrote:

> In D120623#3352640 <https://reviews.llvm.org/D120623#3352640>, @mstorsjo wrote:
>
>> @rnk What do you think about the test in https://github.com/llvm/llvm-project/blob/main/llvm/utils/lit/tests/Inputs/shtest-shell/stdout-encoding.txt which gets broken by this patch? It was added in https://github.com/llvm/llvm-project/commit/27fdf8a29d1e0740c342d428fa48eda7b088ac8e / D69207 <https://reviews.llvm.org/D69207>. Should we just conclude that we shouldn't print non-ascii to stdout in lit tests and remove that testcase?
>
> What is the failure mode? Does lit behave reasonably when binary output is printed to stdout, or does it through an exception? We should expect developers to do this by accident from time to time, and we should adjust that test to express what we want from lit. I guess I'm saying we should keep the test and update the expectations in the outer test.

The failure mode is pretty nonobvious - it will indeed come up occasionally. When that happens, we get a python exception:

  UNRESOLVED: shtest-shell :: stdout-encoding.txt (1 of 1)
  ******************** TEST 'shtest-shell :: stdout-encoding.txt' FAILED ********************
  Exception during script execution:
  Traceback (most recent call last):
    File "/home/martin/code/llvm-project/llvm/utils/lit/lit/worker.py", line 76, in _execute_test_handle_errors
      result = test.config.test_format.execute(test, lit_config)
    File "/home/martin/code/llvm-project/llvm/utils/lit/lit/formats/shtest.py", line 27, in execute
      return lit.TestRunner.executeShTest(test, litConfig,
    File "/home/martin/code/llvm-project/llvm/utils/lit/lit/TestRunner.py", line 1608, in executeShTest
      return _runShTest(test, litConfig, useExternalSh, script, tmpBase)
    File "/home/martin/code/llvm-project/llvm/utils/lit/lit/TestRunner.py", line 1556, in _runShTest 
      res = runOnce(execdir)
    File "/home/martin/code/llvm-project/llvm/utils/lit/lit/TestRunner.py", line 1535, in runOnce
      res = executeScriptInternal(test, litConfig, tmpBase, script, execdir)
    File "/home/martin/code/llvm-project/llvm/utils/lit/lit/TestRunner.py", line 915, in executeScriptInternal
      exitCode, timeoutInfo = executeShCmd(cmd, shenv, results, timeout=litConfig.maxIndividualTestTime)
    File "/home/martin/code/llvm-project/llvm/utils/lit/lit/TestRunner.py", line 161, in executeShCmd
      finalExitCode = _executeShCmd(cmd, shenv, results, timeoutHelper)
    File "/home/martin/code/llvm-project/llvm/utils/lit/lit/TestRunner.py", line 605, in _executeShCmd
      res = _executeShCmd(cmd.lhs, shenv, results, timeoutHelper)
    File "/home/martin/code/llvm-project/llvm/utils/lit/lit/TestRunner.py", line 593, in _executeShCmd
      return _executeShCmd(cmd.rhs, shenv, results, timeoutHelper)
    File "/home/martin/code/llvm-project/llvm/utils/lit/lit/TestRunner.py", line 816, in _executeShCmd
      procData[-1] = procs[-1].communicate()
    File "/usr/lib/python3.8/subprocess.py", line 1028, in communicate
      stdout, stderr = self._communicate(input, endtime, timeout)
    File "/usr/lib/python3.8/subprocess.py", line 1906, in _communicate
      stdout = self._translate_newlines(stdout, 
    File "/usr/lib/python3.8/subprocess.py", line 905, in _translate_newlines
      data = data.decode(encoding, errors)
  UnicodeDecodeError: 'utf-8' codec can't decode byte 0xff in position 14: invalid start byte
       
  
  ********************
  ******************** 
  Unresolved Tests (1):
    shtest-shell :: stdout-encoding.txt




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120623/new/

https://reviews.llvm.org/D120623