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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits