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

Reply via email to