================
@@ -246,6 +288,58 @@ def test_no_failures_build_failed(self):
             ),
         )
 
+    def test_no_failures_build_failed_ninja_log(self):
+        self.assertEqual(
+            generate_test_report_lib.generate_report(
+                "Foo",
+                1,
+                [
+                    junit_from_xml(
+                        dedent(
+                            """\
+          <?xml version="1.0" encoding="UTF-8"?>
+          <testsuites time="0.00">
+          <testsuite name="Passed" tests="1" failures="0" skipped="0" 
time="0.00">
+          <testcase classname="Bar/test_1" name="test_1" time="0.00"/>
+          </testsuite>
+          </testsuites>"""
+                        )
+                    )
+                ],
+                [
+                    [
+                        "[1/5] test/1.stamp",
+                        "[2/5] test/2.stamp",
+                        "[3/5] test/3.stamp",
+                        "[4/5] test/4.stamp",
+                        "FAILED: test/4.stamp",
+                        "touch test/4.stamp",
+                        "Wow! Close To You!",
+                        "[5/5] test/5.stamp",
+                    ]
+                ],
+            ),
+            (
+                dedent(
+                    """\
+                    # Foo
+
+                    * 1 test passed
----------------
DavidSpickett wrote:

> We can't trivially just report build failures along with test failures 
> because the check-* targets will show up as build failures given they are run 
> through ninja. 

Yes but isn't that exactly what this test output shows us doing, reporting 
"build" failures with test failures? Or are you saying that yes, it does show 
that, but to make it clearer is not trivial?

Which I agree with. Detection of failures of a check-target is reasonable to 
leave for future changes.

My point about this output is it does not include any hint that `test/4.stamp` 
should be looked at. I see that 1 test passed and even if GitHub paints some 
part of the UI red, why would I know to click that bit?

Every time we output at least one `<details>` thing, we should at least have 
the "click on a ... to show more".

(it's a shame the default > character used looks so much like a bullet point)

...and if getting all these bits of text in the right place for each 
combination of build and test failures feels like a pain, that's why we might 
want to think about making the report format more rigid. But only if it's 
hampering development. If you're fine dealing with it as is, no problem.

https://github.com/llvm/llvm-project/pull/152621
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

Reply via email to