Merge branch 'main' into hlsl-firstbitlow

Author: V-FEXrt

.ci/generate_test_report.py

@@ -19,12 +19,13 @@ def junit_from_xml(xml):
 
 class TestReports(unittest.TestCase):
     def test_title_only(self):
-        self.assertEqual(_generate_report("Foo", []), ("", "success"))
+        self.assertEqual(_generate_report("Foo", 0, []), ("", "success"))
 
     def test_no_tests_in_testsuite(self):
         self.assertEqual(
             _generate_report(
                 "Foo",
+                1,
                 [
                     junit_from_xml(
                         dedent(
@@ -45,6 +46,7 @@ def test_no_failures(self):
         self.assertEqual(
             _generate_report(
                 "Foo",
+                0,
                 [
                     junit_from_xml(
                         dedent(
@@ -70,10 +72,51 @@ def test_no_failures(self):
             ),
         )
 
+    def test_no_failures_build_failed(self):
+        self.assertEqual(
+            _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>"""
+                        )
+                    )
+                ],
+                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 passed
+
+              All tests passed but another part of the build **failed**.
+
+              [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_single_file_single_testsuite(self):
         self.assertEqual(
             _generate_report(
                 "Foo",
+                1,
                 [
                     junit_from_xml(
                         dedent(
@@ -166,6 +209,7 @@ def test_report_single_file_multiple_testsuites(self):
         self.assertEqual(
             _generate_report(
                 "ABC and DEF",
+                1,
                 [
                     junit_from_xml(
                         dedent(
@@ -198,6 +242,7 @@ def test_report_multiple_files_multiple_testsuites(self):
         self.assertEqual(
             _generate_report(
                 "ABC and DEF",
+                1,
                 [
                     junit_from_xml(
                         dedent(
@@ -238,6 +283,7 @@ def test_report_dont_list_failures(self):
         self.assertEqual(
             _generate_report(
                 "Foo",
+                1,
                 [
                     junit_from_xml(
                         dedent(
@@ -272,6 +318,7 @@ def test_report_dont_list_failures_link_to_log(self):
         self.assertEqual(
             _generate_report(
                 "Foo",
+                1,
                 [
                     junit_from_xml(
                         dedent(
@@ -312,6 +359,7 @@ def test_report_size_limit(self):
         self.assertEqual(
             _generate_report(
                 "Foo",
+                1,
                 [
                     junit_from_xml(
                         dedent(
@@ -351,12 +399,18 @@ def test_report_size_limit(self):
 # and output will not be.
 def _generate_report(
     title,
+    return_code,
     junit_objects,
     size_limit=1024 * 1024,
     list_failures=True,
     buildkite_info=None,
 ):
     if not junit_objects:
+        # Note that we do not post an empty report, therefore we can ignore a
+        # non-zero return code in situations like this.
+        #
+        # If we were going to post a report, then yes, it would be misleading
+        # to say we succeeded when the final return code was non-zero.
         return ("", "success")
 
     failures = {}
@@ -385,7 +439,11 @@ def _generate_report(
     if not tests_run:
         return ("", None)
 
-    style = "error" if tests_failed else "success"
+    style = "success"
+    # Either tests failed, or all tests passed but something failed to build.
+    if tests_failed or return_code != 0:
+        style = "error"
+
     report = [f"# {title}", ""]
 
     tests_passed = tests_run - tests_skipped - tests_failed
@@ -400,17 +458,17 @@ def plural(num_tests):
     if tests_failed:
         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"
+    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"
 
+    if not list_failures:
         report.extend(
             [
                 "",
@@ -435,11 +493,23 @@ def plural(num_tests):
                         "</details>",
                     ]
                 )
+    elif return_code != 0:
+        # No tests failed but the build was in a failed state. Bring this to the user's
+        # attention.
+        report.extend(
+            [
+                "",
+                "All tests passed but another part of the build **failed**.",
+                "",
+                f"{download_text} the build's log file to see the details.",
+            ]
+        )
 
     report = "\n".join(report)
     if len(report.encode("utf-8")) > size_limit:
         return _generate_report(
             title,
+            return_code,
             junit_objects,
             size_limit,
             list_failures=False,
@@ -449,9 +519,10 @@ def plural(num_tests):
     return report, style
 
 
-def generate_report(title, junit_files, buildkite_info):
+def generate_report(title, return_code, junit_files, buildkite_info):
     return _generate_report(
         title,
+        return_code,
         [JUnitXml.fromfile(p) for p in junit_files],
         buildkite_info=buildkite_info,
     )
@@ -463,6 +534,7 @@ def generate_report(title, junit_files, buildkite_info):
         "title", help="Title of the test report, without Markdown formatting."
     )
     parser.add_argument("context", help="Annotation context to write to.")
+    parser.add_argument("return_code", help="The build's return code.", type=int)
     parser.add_argument("junit_files", help="Paths to JUnit report files.", nargs="*")
     args = parser.parse_args()
 
@@ -477,7 +549,9 @@ def generate_report(title, junit_files, buildkite_info):
     if len(buildkite_info) != len(env_var_names):
         buildkite_info = None
 
-    report, style = generate_report(args.title, args.junit_files, buildkite_info)
+    report, style = generate_report(
+        args.title, args.return_code, args.junit_files, buildkite_info
+    )
 
     if report:
         p = subprocess.Popen(

.ci/metrics/metrics.py

@@ -80,6 +80,18 @@ def get_metrics(github_repo: github.Repository, workflows_to_track: dict[str, in
         completed_at = workflow_jobs[0].completed_at
 
         job_result = int(workflow_jobs[0].conclusion == "success")
+        if job_result:
+            # We still might want to mark the job as a failure if one of the steps
+            # failed. This is required due to use setting continue-on-error in
+            # the premerge pipeline to prevent sending emails while we are
+            # testing the infrastructure.
+            # TODO(boomanaiden154): Remove this once the premerge pipeline is no
+            # longer in a testing state and we can directly assert the workflow
+            # result.
+            for step in workflow_jobs[0].steps:
+                if step.conclusion != "success":
+                    job_result = 0
+                    break
 
         queue_time = started_at - created_at
         run_time = completed_at - started_at

.ci/monolithic-linux.sh

@@ -29,6 +29,8 @@ if [[ -n "${CLEAR_CACHE:-}" ]]; then
 fi
 
 function at-exit {
+  retcode=$?
+
   mkdir -p artifacts
   ccache --print-stats > artifacts/ccache_stats.txt
 
@@ -37,7 +39,7 @@ function at-exit {
   if command -v buildkite-agent 2>&1 >/dev/null
   then
     python3 "${MONOREPO_ROOT}"/.ci/generate_test_report.py ":linux: Linux x64 Test Results" \
-      "linux-x64-test-results" "${BUILD_DIR}"/test-results.*.xml
+      "linux-x64-test-results" $retcode "${BUILD_DIR}"/test-results.*.xml
   fi
 }
 trap at-exit EXIT

.ci/monolithic-windows.sh

@@ -28,6 +28,8 @@ fi
 
 sccache --zero-stats
 function at-exit {
+  retcode=$?
+
   mkdir -p artifacts
   sccache --show-stats >> artifacts/sccache_stats.txt
 
@@ -36,7 +38,7 @@ function at-exit {
   if command -v buildkite-agent 2>&1 >/dev/null
   then
     python "${MONOREPO_ROOT}"/.ci/generate_test_report.py ":windows: Windows x64 Test Results" \
-      "windows-x64-test-results" "${BUILD_DIR}"/test-results.*.xml
+      "windows-x64-test-results" $retcode "${BUILD_DIR}"/test-results.*.xml
   fi
 }
 trap at-exit EXIT

.github/workflows/spirv-tests.yml

@@ -26,4 +26,4 @@ jobs:
       build_target: check-llvm-codegen-spirv
       projects:
       extra_cmake_args: '-DLLVM_TARGETS_TO_BUILD="" -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD="SPIRV" -DLLVM_INCLUDE_SPIRV_TOOLS_TESTS=ON'
-      os_list: '["ubuntu-latest"]'
+      os_list: '["ubuntu-22.04"]'

clang-tools-extra/clang-tidy/ClangTidyCheck.cpp

@@ -163,9 +163,10 @@ void ClangTidyCheck::OptionsView::store<bool>(
   store(Options, LocalName, Value ? StringRef("true") : StringRef("false"));
 }
 
-std::optional<int64_t> ClangTidyCheck::OptionsView::getEnumInt(
-    StringRef LocalName, ArrayRef<NameAndValue> Mapping, bool CheckGlobal,
-    bool IgnoreCase) const {
+std::optional<int64_t>
+ClangTidyCheck::OptionsView::getEnumInt(StringRef LocalName,
+                                        ArrayRef<NameAndValue> Mapping,
+                                        bool CheckGlobal) const {
   if (!CheckGlobal && Context->getOptionsCollector())
     Context->getOptionsCollector()->insert((NamePrefix + LocalName).str());
   auto Iter = CheckGlobal ? findPriorityOption(CheckOptions, NamePrefix,
@@ -178,12 +179,10 @@ std::optional<int64_t> ClangTidyCheck::OptionsView::getEnumInt(
   StringRef Closest;
   unsigned EditDistance = 3;
   for (const auto &NameAndEnum : Mapping) {
-    if (IgnoreCase) {
-      if (Value.equals_insensitive(NameAndEnum.second))
-        return NameAndEnum.first;
-    } else if (Value == NameAndEnum.second) {
+    if (Value == NameAndEnum.second) {
       return NameAndEnum.first;
-    } else if (Value.equals_insensitive(NameAndEnum.second)) {
+    }
+    if (Value.equals_insensitive(NameAndEnum.second)) {
       Closest = NameAndEnum.second;
       EditDistance = 0;
       continue;

clang-tools-extra/clang-tidy/ClangTidyCheck.h

@@ -333,9 +333,9 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
     /// supply the mapping required to convert between ``T`` and a string.
     template <typename T>
     std::enable_if_t<std::is_enum_v<T>, std::optional<T>>
-    get(StringRef LocalName, bool IgnoreCase = false) const {
+    get(StringRef LocalName) const {
       if (std::optional<int64_t> ValueOr =
-              getEnumInt(LocalName, typeEraseMapping<T>(), false, IgnoreCase))
+              getEnumInt(LocalName, typeEraseMapping<T>(), false))
         return static_cast<T>(*ValueOr);
       return std::nullopt;
     }
@@ -353,9 +353,9 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
     /// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to
     /// supply the mapping required to convert between ``T`` and a string.
     template <typename T>
-    std::enable_if_t<std::is_enum_v<T>, T> get(StringRef LocalName, T Default,
-                                               bool IgnoreCase = false) const {
-      return get<T>(LocalName, IgnoreCase).value_or(Default);
+    std::enable_if_t<std::is_enum_v<T>, T> get(StringRef LocalName,
+                                               T Default) const {
+      return get<T>(LocalName).value_or(Default);
     }
 
     /// Read a named option from the ``Context`` and parse it as an
@@ -373,9 +373,9 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
     /// supply the mapping required to convert between ``T`` and a string.
     template <typename T>
     std::enable_if_t<std::is_enum_v<T>, std::optional<T>>
-    getLocalOrGlobal(StringRef LocalName, bool IgnoreCase = false) const {
+    getLocalOrGlobal(StringRef LocalName) const {
       if (std::optional<int64_t> ValueOr =
-              getEnumInt(LocalName, typeEraseMapping<T>(), true, IgnoreCase))
+              getEnumInt(LocalName, typeEraseMapping<T>(), true))
         return static_cast<T>(*ValueOr);
       return std::nullopt;
     }
@@ -394,10 +394,9 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
     /// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to
     /// supply the mapping required to convert between ``T`` and a string.
     template <typename T>
-    std::enable_if_t<std::is_enum_v<T>, T>
-    getLocalOrGlobal(StringRef LocalName, T Default,
-                     bool IgnoreCase = false) const {
-      return getLocalOrGlobal<T>(LocalName, IgnoreCase).value_or(Default);
+    std::enable_if_t<std::is_enum_v<T>, T> getLocalOrGlobal(StringRef LocalName,
+                                                            T Default) const {
+      return getLocalOrGlobal<T>(LocalName).value_or(Default);
     }
 
     /// Stores an option with the check-local name \p LocalName with
@@ -454,7 +453,7 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
 
     std::optional<int64_t> getEnumInt(StringRef LocalName,
                                       ArrayRef<NameAndValue> Mapping,
-                                      bool CheckGlobal, bool IgnoreCase) const;
+                                      bool CheckGlobal) const;
 
     template <typename T>
     std::enable_if_t<std::is_enum_v<T>, std::vector<NameAndValue>>