[PATCH] D43314: [lit] - Allow 1 test to report multiple micro-test results to provide support for microbenchmarks.

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

I think this patch should be merged with https://reviews.llvm.org/D43316 as a first example, and then used in https://reviews.llvm.org/D43319 as a proper example. Once https://reviews.llvm.org/D43316 is approved, https://reviews.llvm.org/D43319 should wait until a few green builds are out on the LNT bots to go, once approved.

Detecting LNT errors in the bots is not a trivial task sometimes and pushing only one behavioural change at a time helps a lot.

I've also added a few comments on the Python code.

Finally, please use `diff -U9999` to show the context before uploading the diff. This is hard to review as it is.

Thanks,
--renato



================
Comment at: utils/lit/lit/main.py:86
+        if test.result.microResults:
+            items = sorted(test.result.microResults.items())
+            for micro_test_name, micro_test in items:
----------------
IIRC Python 2.x and 3.x behave different regarding `.items()`.

Do we force Python 3 for LIT?


================
Comment at: utils/lit/lit/main.py:89
+                print("%s MICRO-TEST '%s RESULTS %s" %
+                         ('*'*3, micro_test_name, '*'*3))
+   
----------------
We already have the **** enclosure for the whole test, I think this is a bit too verbose.

I'd suggest something like `*** MICRO-TEST: micro_test_name`


================
Comment at: utils/lit/lit/main.py:91
+   
+                for metric_name, value in micro_test.metrics.items():
+                    print('    %s:  %s ' % (metric_name, value.format()))
----------------
guard with `if micro_test.metrics:`


================
Comment at: utils/lit/lit/main.py:93
+                    print('    %s:  %s ' % (metric_name, value.format()))
+            print("*" * 10)
+
----------------
Same as above, we don't need more *** :)


================
Comment at: utils/lit/lit/main.py:130
+            for key, micro_test in test.result.microResults.items():
+                micro_full_name = test.getFullName()[:-5] + '/' + key + ".test"
+
----------------
What's with the [:-5] ?


https://reviews.llvm.org/D43314