From 5f1baf7ffc890f984a1d8cb296ea4e18222d23bd Mon Sep 17 00:00:00 2001 From: Bhuvanesh66 Date: Sun, 16 Aug 2026 03:35:21 +0530 Subject: [PATCH] hack/ci: fix log summary for non-int ginkgo suites github_log_summary.py chose its parser from the file name, treating a log as ginkgo only when the path contained "int-". logformatter picks its markup from the log contents instead, so any ginkgo suite whose TEST_NAME does not begin with "int" was parsed by the bats parser. The bindings suite is one of those: it is ginkgo, it runs in the small-tests matrix, and its output is piped through logformatter. Given one and the same ginkgo log, int-local-root-fedora.log.html yields the full failure block while bindings-root-fedora.log.html yields only "1 Failed". A failing bindings run therefore leaves the GitHub step summary with nothing useful in it, which is the one moment the summary exists for. Detect the format from the content, the way logformatter does. The "log-failed" class is only ever emitted on the ginkgo path; the logrus handler emits log levels, never "failed". Summarize every file named on the command line as well, instead of only the first one. The workflow passes a glob, and although each job writes a single html file today, any further files were silently dropped. Add hack/ci/github_log_summary.t, the first test for this script, and run it from .check-self-tests. Guarding the entry point on __main__ is what lets the test import the script at all. Signed-off-by: Bhuvanesh66 --- Makefile | 1 + hack/ci/github_log_summary.py | 26 ++++++++--- hack/ci/github_log_summary.t | 82 +++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 7 deletions(-) create mode 100755 hack/ci/github_log_summary.t diff --git a/Makefile b/Makefile index a8702bc3853..45c8e03142d 100644 --- a/Makefile +++ b/Makefile @@ -286,6 +286,7 @@ help: ## Print this help message hack/ci/pr-removes-fixed-skips.t hack/ci/pr-should-include-tests.t hack/ci/logformatter.t + hack/ci/github_log_summary.t test/system/helpers.t .PHONY: lint diff --git a/hack/ci/github_log_summary.py b/hack/ci/github_log_summary.py index 125530f4ed2..0d8af47398c 100755 --- a/hack/ci/github_log_summary.py +++ b/hack/ci/github_log_summary.py @@ -104,12 +104,20 @@ def handle_endtag(self, tag): +# logformatter decides how to mark up a log by looking at the log contents, +# not at the test name, and only its ginkgo path emits the "log-failed" class. +# Detect the format the same way, so that ginkgo suites which are not named +# "int-" (the bindings suite, for example) are parsed as ginkgo rather than +# falling through to the bats parser. +GINKGO_MARKER = 'class="log-failed"' + + def filter_html_file(file_path): # Read the HTML content with open(file_path, 'r', encoding='utf-8') as f: html_content = f.read() - if 'int-' in file_path: + if GINKGO_MARKER in html_content: parser = GinkgoLogFilterParser() parser.feed(html_content) return parser.results @@ -119,10 +127,14 @@ def filter_html_file(file_path): return [parser.data] -# Running the filter -matching_elements = filter_html_file(sys.argv[1]) +def main(file_paths): + for file_path in file_paths: + for element in filter_html_file(file_path): + print("```") + print(element) + print("```") -for element in matching_elements: - print(f"```") - print(element) - print("```") + +# Running the filter +if __name__ == '__main__': + main(sys.argv[1:]) diff --git a/hack/ci/github_log_summary.t b/hack/ci/github_log_summary.t new file mode 100755 index 00000000000..1667fcabe5a --- /dev/null +++ b/hack/ci/github_log_summary.t @@ -0,0 +1,82 @@ +#!/usr/bin/env python3 +# +# tests for github_log_summary.py +# + +import importlib.util +import os +import tempfile +import unittest + +TESTS_DIR = os.path.dirname(os.path.abspath(__file__)) + +# The tool is a script, not a module, so load it by path. +spec = importlib.util.spec_from_file_location( + "github_log_summary", os.path.join(TESTS_DIR, "github_log_summary.py") +) +github_log_summary = importlib.util.module_from_spec(spec) +spec.loader.exec_module(github_log_summary) + + +# Trimmed-down versions of what logformatter emits. The ginkgo path wraps +# failures in a "log-failed" span inside the "tt" block; the bats path marks +# up each line with a "bats-*" class. +GINKGO_HTML = """
+[+0298s] [FAILED] podman pod correctly sets up PIDNS +[+0298s] expected exit code 0, got 125 +
+""" + +GINKGO_PASSING_HTML = """
+[+0271s] ok, all tests passed +
+""" + +BATS_HTML = """
+not ok 1 podman run +# expected 0, got 125 +
+""" + + +def summarize(html, name): + """Write html to a file called name, then run it through the filter.""" + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, name) + with open(path, "w", encoding="utf-8") as f: + f.write(html) + return github_log_summary.filter_html_file(path) + + +class TestFormatDetection(unittest.TestCase): + def test_ginkgo_int_suite(self): + """The int suite is ginkgo and is detected as such.""" + out = "".join(summarize(GINKGO_HTML, "int-local-root-fedora.log.html")) + self.assertIn("[FAILED] podman pod correctly sets up PIDNS", out) + self.assertIn("expected exit code 0, got 125", out) + + def test_ginkgo_suite_not_named_int(self): + """Ginkgo suites are detected by content, not by the file name. + + The bindings suite is ginkgo but is not called "int-". Keying off the + name meant its failures were parsed with the bats parser, which found + no bats markup and so reported nothing useful. + """ + out = "".join(summarize(GINKGO_HTML, "bindings-root-fedora.log.html")) + self.assertIn("[FAILED] podman pod correctly sets up PIDNS", out) + self.assertIn("expected exit code 0, got 125", out) + + def test_bats_suite(self): + """The bats parser still handles bats logs.""" + out = "".join(summarize(BATS_HTML, "sys-local-root-fedora.log.html")) + self.assertIn("not ok 1 podman run", out) + self.assertIn("expected 0, got 125", out) + + def test_ginkgo_without_failures(self): + """A ginkgo log with no failures has nothing to report.""" + out = "".join(summarize(GINKGO_PASSING_HTML, "int-local-root-fedora.log.html")) + self.assertEqual(out.strip(), "") + + +if __name__ == "__main__": + unittest.main()