[llvm] [llvm][llvm-lit] Add option to create unique result file names if results already exist (PR #112729)

David Spickett via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 17 08:28:15 PDT 2024


https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/112729

When running a build like:
ninja check-clang check-llvm

Prior to my changes you ended up with one results file, in this specific case
Junit XML:
results.xml

This would only include the last set of tests lit ran, which were for llvm.
To get around this, many CI systems will run one check target, move the file
away, then run another.

for target in targets:
  ninja target
  mv results.xml results-${target}.xml

I want to use something like this Buildkite reporting plugin in CI,
which needs to have all the results available:
https://buildkite.com/docs/agent/v3/cli-annotate#using-annotations-to-report-test-results

Modifying CI's build scripts for Windows and Linux is a lot of work.
So my changes instead make lit detect an existing result file and modify the file name
until it finds a unique file name to write to.

Now you will get:
results.xml results.1.xml

This will work for all result file types since I'm doing it in the base Report
class. Now you've got separate files, it's easy to collect them with `<path>/*.xml`.

The number will increment as many times as needed until a useable name
is found.

>From 5ea2d1179d52e18cfe304f710d6b7c4ddaed4935 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Thu, 17 Oct 2024 14:17:19 +0000
Subject: [PATCH 1/2] [llvm][llvm-lit] Add a base class for reports that are
 written to files

This is to later allow me to handle the choosing of the filename
in one place, so that we can write unique files across multiple
runs.
---
 llvm/utils/lit/lit/reports.py | 61 ++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 30 deletions(-)

diff --git a/llvm/utils/lit/lit/reports.py b/llvm/utils/lit/lit/reports.py
index d2d719b076bc70..090426f982debd 100755
--- a/llvm/utils/lit/lit/reports.py
+++ b/llvm/utils/lit/lit/reports.py
@@ -1,3 +1,4 @@
+import abc
 import base64
 import datetime
 import itertools
@@ -14,11 +15,22 @@ def by_suite_and_test_path(test):
     return (test.suite.name, id(test.suite), test.path_in_suite)
 
 
-class JsonReport(object):
+class Report(object):
     def __init__(self, output_file):
         self.output_file = output_file
 
     def write_results(self, tests, elapsed):
+        with open(self.output_file, "w") as file:
+            self._write_results_to_file(tests, elapsed, file)
+
+    @abc.abstractmethod
+    def _write_results_to_file(self, tests, elapsed, file):
+        """Write test results to the file object "file"."""
+        pass
+
+
+class JsonReport(Report):
+    def _write_results_to_file(self, tests, elapsed, file):
         unexecuted_codes = {lit.Test.EXCLUDED, lit.Test.SKIPPED}
         tests = [t for t in tests if t.result.code not in unexecuted_codes]
         # Construct the data we will write.
@@ -67,9 +79,8 @@ def write_results(self, tests, elapsed):
 
             tests_data.append(test_data)
 
-        with open(self.output_file, "w") as file:
-            json.dump(data, file, indent=2, sort_keys=True)
-            file.write("\n")
+        json.dump(data, file, indent=2, sort_keys=True)
+        file.write("\n")
 
 
 _invalid_xml_chars_dict = {
@@ -88,21 +99,18 @@ def remove_invalid_xml_chars(s):
     return s.translate(_invalid_xml_chars_dict)
 
 
-class XunitReport(object):
-    def __init__(self, output_file):
-        self.output_file = output_file
-        self.skipped_codes = {lit.Test.EXCLUDED, lit.Test.SKIPPED, lit.Test.UNSUPPORTED}
+class XunitReport(Report):
+    skipped_codes = {lit.Test.EXCLUDED, lit.Test.SKIPPED, lit.Test.UNSUPPORTED}
 
-    def write_results(self, tests, elapsed):
+    def _write_results_to_file(self, tests, elapsed, file):
         tests.sort(key=by_suite_and_test_path)
         tests_by_suite = itertools.groupby(tests, lambda t: t.suite)
 
-        with open(self.output_file, "w") as file:
-            file.write('<?xml version="1.0" encoding="UTF-8"?>\n')
-            file.write('<testsuites time="{time:.2f}">\n'.format(time=elapsed))
-            for suite, test_iter in tests_by_suite:
-                self._write_testsuite(file, suite, list(test_iter))
-            file.write("</testsuites>\n")
+        file.write('<?xml version="1.0" encoding="UTF-8"?>\n')
+        file.write('<testsuites time="{time:.2f}">\n'.format(time=elapsed))
+        for suite, test_iter in tests_by_suite:
+            self._write_testsuite(file, suite, list(test_iter))
+        file.write("</testsuites>\n")
 
     def _write_testsuite(self, file, suite, tests):
         skipped = 0
@@ -206,11 +214,8 @@ def gen_resultdb_test_entry(
     return test_data
 
 
-class ResultDBReport(object):
-    def __init__(self, output_file):
-        self.output_file = output_file
-
-    def write_results(self, tests, elapsed):
+class ResultDBReport(Report):
+    def _write_results_to_file(self, tests, elapsed, file):
         unexecuted_codes = {lit.Test.EXCLUDED, lit.Test.SKIPPED}
         tests = [t for t in tests if t.result.code not in unexecuted_codes]
         data = {}
@@ -249,17 +254,14 @@ def write_results(self, tests, elapsed):
                         )
                     )
 
-        with open(self.output_file, "w") as file:
-            json.dump(data, file, indent=2, sort_keys=True)
-            file.write("\n")
+        json.dump(data, file, indent=2, sort_keys=True)
+        file.write("\n")
 
 
-class TimeTraceReport(object):
-    def __init__(self, output_file):
-        self.output_file = output_file
-        self.skipped_codes = {lit.Test.EXCLUDED, lit.Test.SKIPPED, lit.Test.UNSUPPORTED}
+class TimeTraceReport(Report):
+    skipped_codes = {lit.Test.EXCLUDED, lit.Test.SKIPPED, lit.Test.UNSUPPORTED}
 
-    def write_results(self, tests, elapsed):
+    def _write_results_to_file(self, tests, elapsed, file):
         # Find when first test started so we can make start times relative.
         first_start_time = min([t.result.start for t in tests])
         events = [
@@ -270,8 +272,7 @@ def write_results(self, tests, elapsed):
 
         json_data = {"traceEvents": events}
 
-        with open(self.output_file, "w") as time_trace_file:
-            json.dump(json_data, time_trace_file, indent=2, sort_keys=True)
+        json.dump(json_data, time_trace_file, indent=2, sort_keys=True)
 
     def _get_test_event(self, test, first_start_time):
         test_name = test.getFullName()

>From aae07c3ef2aa230d53c6620d5c43bc7350b7b4bf Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Thu, 17 Oct 2024 14:35:01 +0000
Subject: [PATCH 2/2] [llvm][llvm-lit] Add option to create unique result file
 names if results already exist

When running a build like:
ninja check-clang check-llvm

Prior to my changes you ended up with one results file, in this specific case
Junit XML:
results.xml

This would only include the last set of tests lit ran, which were for llvm.
To get around this, many CI systems will run one check target, move the file
away, then run another.

for target in targets:
  ninja target
  mv results.xml results-${target}.xml

I want to use something like this Buildkite reporting plugin in CI,
which needs to have all the results available:
https://buildkite.com/docs/agent/v3/cli-annotate#using-annotations-to-report-test-results

Modifying CI's build scripts for Windows and Linux is a lot of work.
So my changes instead make lit detect an existing result file and modify the file name
until it finds a unique file name to write to.

Now you will get:
results.xml results.1.xml

This will work for all result file types since I'm doing it in the base Report
class. Now you've got separate files, it's easy to collect them with `<path>/*.xml`.

The number will increment as many times as needed until a useable name
is found.
---
 llvm/utils/lit/lit/cl_arguments.py         | 28 +++++++++++++++-------
 llvm/utils/lit/lit/reports.py              | 26 ++++++++++++++++++--
 llvm/utils/lit/tests/unique-output-file.py | 28 ++++++++++++++++++++++
 3 files changed, 72 insertions(+), 10 deletions(-)
 create mode 100644 llvm/utils/lit/tests/unique-output-file.py

diff --git a/llvm/utils/lit/lit/cl_arguments.py b/llvm/utils/lit/lit/cl_arguments.py
index 5ccae4be096796..3b11342dec2162 100644
--- a/llvm/utils/lit/lit/cl_arguments.py
+++ b/llvm/utils/lit/lit/cl_arguments.py
@@ -175,6 +175,13 @@ def parse_args():
         type=lit.reports.TimeTraceReport,
         help="Write Chrome tracing compatible JSON to the specified file",
     )
+    execution_group.add_argument(
+        "--use-unique-output-file-name",
+        help="When enabled, lit will not overwrite existing test report files. "
+        "Instead it will modify the file name until it finds a file name "
+        "that does not already exist. [Default: Off]",
+        action="store_true",
+    )
     execution_group.add_argument(
         "--timeout",
         dest="maxIndividualTestTime",
@@ -332,16 +339,21 @@ def parse_args():
     else:
         opts.shard = None
 
-    opts.reports = filter(
-        None,
-        [
-            opts.output,
-            opts.xunit_xml_output,
-            opts.resultdb_output,
-            opts.time_trace_output,
-        ],
+    opts.reports = list(
+        filter(
+            None,
+            [
+                opts.output,
+                opts.xunit_xml_output,
+                opts.resultdb_output,
+                opts.time_trace_output,
+            ],
+        )
     )
 
+    for report in opts.reports:
+        report.use_unique_output_file_name = opts.use_unique_output_file_name
+
     return opts
 
 
diff --git a/llvm/utils/lit/lit/reports.py b/llvm/utils/lit/lit/reports.py
index 090426f982debd..c2a0239e360f37 100755
--- a/llvm/utils/lit/lit/reports.py
+++ b/llvm/utils/lit/lit/reports.py
@@ -3,6 +3,7 @@
 import datetime
 import itertools
 import json
+import os
 
 from xml.sax.saxutils import quoteattr as quo
 
@@ -18,10 +19,31 @@ def by_suite_and_test_path(test):
 class Report(object):
     def __init__(self, output_file):
         self.output_file = output_file
+        # Set by the option parser later.
+        self.use_unique_output_file_name = False
 
     def write_results(self, tests, elapsed):
-        with open(self.output_file, "w") as file:
-            self._write_results_to_file(tests, elapsed, file)
+        if self.use_unique_output_file_name:
+            file = None
+            filepath = self.output_file
+            attempt = 0
+            while file is None:
+                try:
+                    file = open(filepath, "x")
+                except FileExistsError:
+                    attempt += 1
+                    # If there is an extension insert before that because most
+                    # glob patterns for these will be '*.extension'. Otherwise
+                    # add to the end of the path.
+                    path, ext = os.path.splitext(self.output_file)
+                    filepath = path + f".{attempt}" + ext
+
+            with file:
+                self._write_results_to_file(tests, elapsed, file)
+        else:
+            # Overwrite if the results already exist.
+            with open(self.output_file, "w") as file:
+                self._write_results_to_file(tests, elapsed, file)
 
     @abc.abstractmethod
     def _write_results_to_file(self, tests, elapsed, file):
diff --git a/llvm/utils/lit/tests/unique-output-file.py b/llvm/utils/lit/tests/unique-output-file.py
new file mode 100644
index 00000000000000..e0ce21aebf6950
--- /dev/null
+++ b/llvm/utils/lit/tests/unique-output-file.py
@@ -0,0 +1,28 @@
+# Check that lit will not overwrite existing result files when given
+# --use-unique-output-file-name.
+
+# Files are overwritten without the option.
+# RUN: rm -rf %t.xunit*.xml
+# RUN: echo "test" > %t.xunit.xml
+# RUN: not %{lit} --xunit-xml-output %t.xunit.xml %{inputs}/xunit-output
+# RUN: FileCheck < %t.xunit.xml %s --check-prefix=NEW
+
+# RUN: rm -rf %t.xunit*.xml
+# RUN: echo "test" > %t.xunit.xml
+# Files should not be overwritten with the option.
+# RUN: not %{lit} --xunit-xml-output %t.xunit.xml --use-unique-output-file-name %{inputs}/xunit-output
+# RUN: FileCheck < %t.xunit.xml %s --check-prefix=EXISTING
+# EXISTING: test
+# Results in a new file with "1" added.
+# RUN: FileCheck < %t.xunit.1.xml %s --check-prefix=NEW
+# NEW:      <?xml version="1.0" encoding="UTF-8"?>
+# NEW-NEXT: <testsuites time="{{[0-9.]+}}">
+# (assuming that other tests check the whole contents of the file)
+
+# The number should increment as many times as needed.
+# RUN: touch %t.xunit.2.xml
+# RUN: touch %t.xunit.3.xml
+# RUN: touch %t.xunit.4.xml
+
+# RUN: not %{lit} --xunit-xml-output %t.xunit.xml --use-unique-output-file-name %{inputs}/xunit-output
+# RUN: FileCheck < %t.xunit.5.xml %s --check-prefix=NEW
\ No newline at end of file



More information about the llvm-commits mailing list