[PATCH] D143519: [lit] [PATCH 2/2] Add "--reduced-xunit-report" option

[Julian Lettner via Phabricator via llvm-commits]

yln added a subscriber: jdenny.
yln added a comment.

Initially, my main question was: would this functionality make sense for reports other than the xml one too?  Probably not, because the use case is pretty specific, right?  Anyways, I don't think we need to implement that now and leave that work to whoever needs it.

- 3 small nits
- Is it possible to write a test for this?

I am okay with the overall motivation considering it's gated behind a flag.

@jdenny: Comments on the larger trade-off of having this feature?



================
Comment at: llvm/utils/lit/lit/cl_arguments.py:121
             help="Write XUnit-compatible XML test reports to the specified file")
+    execution_group.add_argument("--reduced-xunit-report",
+            dest="reducedXUnitReport",
----------------
Would something like "report/output failures only" be a more precise flag name?


================
Comment at: llvm/utils/lit/lit/cl_arguments.py:122
+    execution_group.add_argument("--reduced-xunit-report",
+            dest="reducedXUnitReport",
+            help="When writing an XUNit test report, do not include results for "
----------------
I would prefer to omit the `dest=...` parameter and let ArgParse pick a Python-idiomatic field name.


================
Comment at: llvm/utils/lit/lit/main.py:121
     for report in opts.reports:
-        report.write_results(tests_for_report, elapsed)
+        report.write_results(tests_for_report, elapsed, opts)
 
----------------
Please pass `opts.reduced_report` instead of the entire options object here.


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

https://reviews.llvm.org/D143519