[Lldb-commits] [PATCH] D81697: Add support for batch-testing to the LLDB testsuite.

Wed Jul 8 04:49:17 PDT 2020

labath added a comment.

In D81697#2138024 <https://reviews.llvm.org/D81697#2138024>, @bbli wrote:

> F12311388: image.png <https://reviews.llvm.org/F12311388>
>
> Hi, so I think I got the fix working. Attached is a screenshot of the new output, with title "Build Command Stdout Ouput". Should I submit a new pull request for this?

Yes, that looks better. Please create a patch for that. It might be even better if the stdout+stderr contents came as a single stream (as if run by `cmd 2>&1`). That way the error messages on stderr will appear right next to the compiler invocation (which goes to stdout). I believe this could be achieved by setting `stderr` to `STDOUT` (instead of `PIPE`).

> Also just wondering,  it seems you guys have added a lldb_extensions dictionary to the `CalledProcessError` class. Was this monkey-patched in, because I don't see `subprocess` as one of the vendored third party libraries in the repo?

It looks like this just uses the lack of access protections in python to create a new field in a random object (lldbtest.py:442). A cleaner approach would be to create a new class for this exception (perhaps by subclassing CalledProcessError).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81697