[Lldb-commits] [PATCH] D11816: Allow dosep.py to print full output of dotest.py, even when dotest succeeds.

[Chaoren Lin via lldb-commits]

chaoren added inline comments.

================
Comment at: test/dosep.py:82
@@ +81,3 @@
+    with output_lock:
+        print >> sys.stderr, stdout
+        print >> sys.stderr, "[%s] FAILED" % name
----------------
Why did you move this here? Could you please move stderr here too? It seems weird stderr is labeled but stdout isn't.

================
Comment at: test/dosep.py:102
@@ -89,2 +101,3 @@
         test_counter.value += 1
+    sys.stdout.flush()
     sys.stderr.flush()
----------------
This and `sys.stderr.flush()` should be under the `with output_lock:`. My mistake. Sorry.

================
Comment at: test/dosep.py:142
@@ -128,2 +141,3 @@
     passes, failures = parse_test_results(output)
-    update_status(name, command, output if exit_status != 0 else None)
+    if exit_status == 0:
+        report_test_pass(name, output[1])
----------------
Could you please do:

```
if exit_status != 0:
    report_test_failure(...)
elif output_on_success:
    report_test_pass(...)
```

so it's completely silent on success?

================
Comment at: test/dosep.py:143
@@ +142,3 @@
+    if exit_status == 0:
+        report_test_pass(name, output[1])
+    else:
----------------
Do you mean `output[0]`? For consistency, should we print stderr for success too? Even if it's mostly empty.


http://reviews.llvm.org/D11816