Skip to content

[ci] Include a log download link when test report is truncated #117985

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

DavidSpickett
Copy link
Collaborator

Now "Download" will be a link to the file so people don't have to know
to open the build tab and find the download button.

This is a URL from a real build:
https://buildkite.com/organizations/llvm-project/pipelines/github-pull-requests/builds/123979/jobs/01937132-0fc3-4c95-a884-2fc0048cb9a7/download.txt
And this is how we can build it:
https://buildkite.com/organizations/{BUILDKITE_ORGANIZATION_SLUG}/pipelines/{BUILDKITE_PIPELINE_SLUG}/builds/{BUILDKITE_BUILD_NUMBER}/jobs/{BUILDKITE_JOB_ID}/download.txt

Given these env vars that were set in that job:
BUILDKITE_ORGANIZATION_SLUG="llvm-project"
BUILDKITE_PIPELINE_SLUG="github-pull-requests"
BUILDKITE_BUILD_NUMBER="123979"
BUILDKITE_JOB_ID="01937132-0fc3-4c95-a884-2fc0048cb9a7"

In theory these will always be available but:

  1. Rather safe than sorry with this script, I don't want to make a passing
    build a failure because this script failed.
  2. It would get very annoying if you had to set all these to test
    the script locally.

Copy link

github-actions bot commented Nov 28, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Now "Download" will be a link to the file so people don't have to know
to open the build tab and find the download button.

This is a URL from a real build:
https://buildkite.com/organizations/llvm-project/pipelines/github-pull-requests/builds/123979/jobs/01937132-0fc3-4c95-a884-2fc0048cb9a7/download.txt
And this is how we can build it:
https://buildkite.com/organizations/{BUILDKITE_ORGANIZATION_SLUG}/pipelines/{BUILDKITE_PIPELINE_SLUG}/builds/{BUILDKITE_BUILD_NUMBER}/jobs/{BUILDKITE_JOB_ID}/download.txt

Given these env vars that were set in that job:
BUILDKITE_ORGANIZATION_SLUG="llvm-project"
BUILDKITE_PIPELINE_SLUG="github-pull-requests"
BUILDKITE_BUILD_NUMBER="123979"
BUILDKITE_JOB_ID="01937132-0fc3-4c95-a884-2fc0048cb9a7"

In theory these will always be available but:
1. Rather safe than sorry with this script, I don't want to make a passing
   build a failure because this script failed.
2. It would get very annoying if you had to set all these to test
   the script locally.
@DavidSpickett DavidSpickett force-pushed the log-file-link branch 2 times, most recently from c9dd2cf to 7e9e3f5 Compare November 28, 2024 13:06
@DavidSpickett
Copy link
Collaborator Author

@DavidSpickett DavidSpickett marked this pull request as ready for review November 28, 2024 13:07
@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2024

@llvm/pr-subscribers-github-workflow

Author: David Spickett (DavidSpickett)

Changes

Now "Download" will be a link to the file so people don't have to know
to open the build tab and find the download button.

This is a URL from a real build:
https://buildkite.com/organizations/llvm-project/pipelines/github-pull-requests/builds/123979/jobs/01937132-0fc3-4c95-a884-2fc0048cb9a7/download.txt
And this is how we can build it:
https://buildkite.com/organizations/{BUILDKITE_ORGANIZATION_SLUG}/pipelines/{BUILDKITE_PIPELINE_SLUG}/builds/{BUILDKITE_BUILD_NUMBER}/jobs/{BUILDKITE_JOB_ID}/download.txt

Given these env vars that were set in that job:
BUILDKITE_ORGANIZATION_SLUG="llvm-project"
BUILDKITE_PIPELINE_SLUG="github-pull-requests"
BUILDKITE_BUILD_NUMBER="123979"
BUILDKITE_JOB_ID="01937132-0fc3-4c95-a884-2fc0048cb9a7"

In theory these will always be available but:

  1. Rather safe than sorry with this script, I don't want to make a passing
    build a failure because this script failed.
  2. It would get very annoying if you had to set all these to test
    the script locally.

Full diff: https://github.com/llvm/llvm-project/pull/117985.diff

1 Files Affected:

  • (modified) .ci/generate_test_report.py (+84-6)
diff --git a/.ci/generate_test_report.py b/.ci/generate_test_report.py
index c44936b19dab98..ff601a0cde1063 100644
--- a/.ci/generate_test_report.py
+++ b/.ci/generate_test_report.py
@@ -5,6 +5,7 @@
 # python3 -m unittest discover -p generate_test_report.py
 
 import argparse
+import os
 import subprocess
 import unittest
 from io import StringIO
@@ -267,6 +268,46 @@ def test_report_dont_list_failures(self):
             ),
         )
 
+    def test_report_dont_list_failures_link_to_log(self):
+        self.assertEqual(
+            _generate_report(
+                "Foo",
+                [
+                    junit_from_xml(
+                        dedent(
+                            """\
+          <?xml version="1.0" encoding="UTF-8"?>
+          <testsuites time="0.02">
+          <testsuite name="Bar" tests="1" failures="1" skipped="0" time="0.02">
+          <testcase classname="Bar/test_1" name="test_1" time="0.02">
+            <failure><![CDATA[Output goes here]]></failure>
+          </testcase>
+          </testsuite>
+          </testsuites>"""
+                        )
+                    )
+                ],
+                list_failures=False,
+                buildkite_info={
+                    "BUILDKITE_ORGANIZATION_SLUG": "organization_slug",
+                    "BUILDKITE_PIPELINE_SLUG": "pipeline_slug",
+                    "BUILDKITE_BUILD_NUMBER": "build_number",
+                    "BUILDKITE_JOB_ID": "job_id",
+                },
+            ),
+            (
+                dedent(
+                    """\
+          # Foo
+
+          * 1 test failed
+
+          Failed tests and their output was too large to report. [Download](https://buildkite.com/organizations/organization_slug/pipelines/pipeline_slug/builds/build_number/jobs/job_id/download.txt) the build's log file to see the details."""
+                ),
+                "error",
+            ),
+        )
+
     def test_report_size_limit(self):
         self.assertEqual(
             _generate_report(
@@ -308,7 +349,13 @@ def test_report_size_limit(self):
 # listed. This minimal report will always fit into an annotation.
 # If include failures is False, total number of test will be reported but their names
 # and output will not be.
-def _generate_report(title, junit_objects, size_limit=1024 * 1024, list_failures=True):
+def _generate_report(
+    title,
+    junit_objects,
+    size_limit=1024 * 1024,
+    list_failures=True,
+    buildkite_info=None,
+):
     if not junit_objects:
         return ("", "success")
 
@@ -354,11 +401,21 @@ def plural(num_tests):
         report.append(f"* {tests_failed} {plural(tests_failed)} failed")
 
     if not list_failures:
+        if buildkite_info is not None:
+            log_url = (
+                "https://buildkite.com/organizations/{BUILDKITE_ORGANIZATION_SLUG}/"
+                "pipelines/{BUILDKITE_PIPELINE_SLUG}/builds/{BUILDKITE_BUILD_NUMBER}/"
+                "jobs/{BUILDKITE_JOB_ID}/download.txt".format(**buildkite_info)
+            )
+            download_text = f"[Download]({log_url})"
+        else:
+            download_text = "Download"
+
         report.extend(
             [
                 "",
                 "Failed tests and their output was too large to report. "
-                "Download the build's log file to see the details.",
+                f"{download_text} the build's log file to see the details.",
             ]
         )
     elif failures:
@@ -381,13 +438,23 @@ def plural(num_tests):
 
     report = "\n".join(report)
     if len(report.encode("utf-8")) > size_limit:
-        return _generate_report(title, junit_objects, size_limit, list_failures=False)
+        return _generate_report(
+            title,
+            junit_objects,
+            size_limit,
+            list_failures=False,
+            buildkite_info=buildkite_info,
+        )
 
     return report, style
 
 
-def generate_report(title, junit_files):
-    return _generate_report(title, [JUnitXml.fromfile(p) for p in junit_files])
+def generate_report(title, junit_files, buildkite_info):
+    return _generate_report(
+        title,
+        [JUnitXml.fromfile(p) for p in junit_files],
+        buildkite_info=buildkite_info,
+    )
 
 
 if __name__ == "__main__":
@@ -399,7 +466,18 @@ def generate_report(title, junit_files):
     parser.add_argument("junit_files", help="Paths to JUnit report files.", nargs="*")
     args = parser.parse_args()
 
-    report, style = generate_report(args.title, args.junit_files)
+    # All of these are required to build a link to download the log file.
+    env_var_names = [
+        "BUILDKITE_ORGANIZATION_SLUG",
+        "BUILDKITE_PIPELINE_SLUG",
+        "BUILDKITE_BUILD_NUMBER",
+        "BUILDKITE_JOB_ID",
+    ]
+    buildkite_info = {k: v for k, v in os.environ.items() if k in env_var_names}
+    if len(buildkite_info) != len(env_var_names):
+        buildkite_info = None
+
+    report, style = generate_report(args.title, args.junit_files, buildkite_info)
 
     if report:
         p = subprocess.Popen(

@DavidSpickett
Copy link
Collaborator Author

ping!

Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Sorry for taking so long on the review. Thanks for the improvement!

@DavidSpickett DavidSpickett merged commit 71fd528 into llvm:main Dec 11, 2024
9 checks passed
@DavidSpickett DavidSpickett deleted the log-file-link branch December 11, 2024 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants