[Lldb-commits] [PATCH] D83425: [lldb] add printing of stdout compile errors to lldbsuite

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/packages/Python/lldbsuite/test_event/build_exception.py:8
         self.build_error = called_process_error.lldb_extensions.get(
-            "stderr_content", "<error output unavailable>")
+            "stderr_content", "<error output unavailable>").decode("utf-8") + called_process_error.lldb_extensions.get("stdout_content","<stdout output unavailable>").decode("utf-8")
 
----------------
This `.decode` call will be incorrect on python3 if `lldb_extensions` ever does not contain the `stderr_content` field. (That's because `"<error output unavailable>"`, a string object, does not have a `decode` method -- only `bytes` objects can be decoded in python3.

By the looks of things, stderr_content will always be filled in, so you should be able to just replace this with direct member access (`called_process_error.lldb_extensions.stderr_content.decode(...)`).

Also, if I'm not mistaken, this will just concatenate stdout and stderr without any sort of delimiters, or caring about the order in which they are produced. That could end up being confusing because the compilers stderr will come before the compiler command which wrote it. If we're not going to be merging the two streams in the proper sequence (I described one way in <https://reviews.llvm.org/D81697#2138665>), then it would be better to prefix each stream with it's own header (like you've done in <https://reviews.llvm.org/file/data/wxosaassniqt7keadc2v/PHID-FILE-6o2ea7gsgnrznam5k4ua/image.png>), so that it is clear that one is looking at two independent streams.


================
Comment at: lldb/packages/Python/lldbsuite/test_event/formatter/xunit.py:253-269
     def _handle_error_build(self, test_event):
         """Handles a test error.
         @param test_event the test event to handle.
         """
         message = self._replace_invalid_xml(test_event["issue_message"])
         build_issue_description = self._replace_invalid_xml(
             BuildError.format_build_error(
----------------
I believe this file is pretty much unused now. I've created D83545 to remove it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83425