[all-commits] [llvm/llvm-project] 080fb3: [lit] Clean up internal shell parse errors with Sc...

[Joel E. Denny via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 080fb3e5b73bcfd90f11335ff3d175cf3199f814
      https://github.com/llvm/llvm-project/commit/080fb3e5b73bcfd90f11335ff3d175cf3199f814
  Author: Joel E. Denny <jdenny.ornl at gmail.com>
  Date:   2023-10-20 (Fri, 20 Oct 2023)

  Changed paths:
    M libcxx/utils/libcxx/test/format.py
    M llvm/utils/lit/lit/TestRunner.py
    M llvm/utils/lit/tests/shtest-shell.py

  Log Message:
  -----------
  [lit] Clean up internal shell parse errors with ScriptFatal (#68496)

Without this patch, the functions `executeScriptInternal` and thus
`runOnce` in `llvm/utils/lit/lit/TestRunner.py` return either a tuple like
`(out, err, exitCode, timeoutInfo)` or a `lit.Test.Result` object. They
return the latter only when there's a lit internal shell parse error in
a RUN line. In my opinion, a more straight-forward way to handle
exceptional cases like that is to use python exceptions.

For that purpose, this patch introduces `ScriptFatal`. Thus, this patch
changes `executeScriptInternal` to always either return the tuple or
raise the `ScriptFatal` exception. It updates `runOnce` and
`libcxx/utils/libcxx/test/format.py` to catch the exception rather than
check for the special return type.

This patch also changes `runOnce` to convert the exception to a
`Test.UNRESOLVED` result instead of `TEST.FAIL`. The former is the
proper result for such a malformed test, for which a rerun (given an
`ALLOW_RETRIES:`) serves no purpose. There are at least two benefits
from this change. First, `_runShTest` no longer has to specially and
cryptically handle this case to avoid unnecessary reruns. Second, an
`XFAIL:` directive no longer hides such a failure [as we saw
previously](https://reviews.llvm.org/D154987#4501125).

To facilitate the `_runShTest` change, this patch inserts the internal
shell parse error diagnostic into the format of the test's normal debug
output rather than suppressing the latter entirely. That change is also
important for [D154987](https://reviews.llvm.org/D154987), which
proposes to reuse `ScriptFatal` for python compile errors in PYTHON
lines or in `config.prologue`. In that case, the diagnostic might follow
debugging output from the test's previous RUN or PYTHON lines, so
suppressing the normal debug output would lose information.