Record warnings from the Xcode rules as structured information in dummy actions so that we can test them.

allevato · swiple-rules-gardener · commit 0f6c6a3107c2 · 2024-05-08T11:13:32.000-07:00
PiperOrigin-RevId: 631862483
diff --git a/test/test_helpers.bzl b/test/test_helpers.bzl
@@ -26,6 +26,47 @@ FIXTURE_TAGS = [
     "manual",
 ]
 
+def assert_warning(env, msg_id, data = None):
+    """Asserts that a warning was printed.
+
+    The logic in this helper is specifically meant to handle warnings printed by
+    `xcode_config.bzl`, because we want to preserve the behavior of the original
+    rules and their tests.
+
+    Args:
+        env: The analysis test environment.
+        msg_id: The fixed identifier of the warning message.
+        data: The semicolon-delimited, sorted by key, `key=value` string
+            representation of the format arguments for the message, if any.
+    """
+    expected = "Warning:{}".format(msg_id)
+    if data:
+        expected += ":{}".format(data)
+
+    found_warnings = []
+
+    actions = analysistest.target_actions(env)
+    for action in actions:
+        mnemonic = action.mnemonic
+        if mnemonic == expected:
+            return
+        if mnemonic.startswith("Warning:"):
+            found_warnings.append(mnemonic)
+
+    found_warnings_msg = ""
+    if found_warnings:
+        found_warnings_msg = "; the following warnings were emitted:\n{}".format(
+            "\n".join(found_warnings),
+        )
+
+    analysistest.fail(
+        env,
+        "Expected warning '{}' was not emitted{}".format(
+            expected,
+            found_warnings_msg,
+        ),
+    )
+
 def find_action(env, mnemonic):
     """Finds the first action with the given mnemonic in the target under test.
 
diff --git a/test/xcode_config_test.bzl b/test/xcode_config_test.bzl
@@ -21,6 +21,7 @@ load(
 )
 load(
     "@build_bazel_apple_support//xcode:xcode_config.bzl",
+    "UNAVAILABLE_XCODE_MESSAGE",
     "xcode_config",
 )
 load(
@@ -35,7 +36,13 @@ load(
     "@build_bazel_apple_support//xcode/private:providers.bzl",
     "XcodeVersionPropertiesInfo",
 )  # buildifier: disable=bzl-visibility
-load(":test_helpers.bzl", "FIXTURE_TAGS", "find_action", "make_all_tests")
+load(
+    ":test_helpers.bzl",
+    "FIXTURE_TAGS",
+    "assert_warning",
+    "find_action",
+    "make_all_tests",
+)
 
 visibility("private")
 
@@ -333,6 +340,12 @@ def _prefers_flag_over_mutually_available_test_impl(ctx):
     asserts.true(env, "no-local" in xcode_version_info.execution_info())
     asserts.true(env, "supports-xcode-requirements-set" in xcode_version_info.execution_info())
 
+    assert_warning(
+        env,
+        "version_not_available_locally",
+        "command={};local_versions=8.4;version=5.1.2".format(UNAVAILABLE_XCODE_MESSAGE),
+    )
+
     return analysistest.end(env)
 
 _prefers_flag_over_mutually_available_test = analysistest.make(
@@ -369,16 +382,18 @@ def _warn_with_explicit_local_only_version_test_impl(ctx):
     target_under_test = analysistest.target_under_test(env)
     xcode_version_info = target_under_test[apple_common.XcodeVersionConfig]
 
-    # TODO: b/311385128 - Once we move the rules to apple_support, hack up
-    # something that would let us actually test the warning messages. We can't
-    # test `print`.
-
     asserts.equals(env, "8.4", str(xcode_version_info.xcode_version()))
     asserts.equals(env, "local", xcode_version_info.availability())
     asserts.true(env, "requires-darwin" in xcode_version_info.execution_info())
     asserts.true(env, "no-remote" in xcode_version_info.execution_info())
     asserts.true(env, "supports-xcode-requirements-set" in xcode_version_info.execution_info())
 
+    assert_warning(
+        env,
+        "explicit_version_not_available_remotely",
+        "version=8.4",
+    )
+
     return analysistest.end(env)
 
 _warn_with_explicit_local_only_version_test = analysistest.make(
@@ -415,16 +430,18 @@ def _prefer_local_default_if_no_mutual_no_flag_different_main_version_test_impl(
     target_under_test = analysistest.target_under_test(env)
     xcode_version_info = target_under_test[apple_common.XcodeVersionConfig]
 
-    # TODO: b/311385128 - Once we move the rules to apple_support, hack up
-    # something that would let us actually test the warning messages. We can't
-    # test `print`.
-
     asserts.equals(env, "8.4", str(xcode_version_info.xcode_version()))
     asserts.equals(env, "local", xcode_version_info.availability())
     asserts.true(env, "requires-darwin" in xcode_version_info.execution_info())
     asserts.true(env, "no-remote" in xcode_version_info.execution_info())
     asserts.true(env, "supports-xcode-requirements-set" in xcode_version_info.execution_info())
 
+    assert_warning(
+        env,
+        "local_default_not_available_remotely",
+        "local_version=8.4;remote_versions=5.1.2",
+    )
+
     return analysistest.end(env)
 
 _prefer_local_default_if_no_mutual_no_flag_different_main_version_test = analysistest.make(
@@ -460,16 +477,18 @@ def _prefer_local_default_if_no_mutual_no_flag_different_build_alias_test_impl(c
     target_under_test = analysistest.target_under_test(env)
     xcode_version_info = target_under_test[apple_common.XcodeVersionConfig]
 
-    # TODO: b/311385128 - Once we move the rules to apple_support, hack up
-    # something that would let us actually test the warning messages. We can't
-    # test `print`.
-
     asserts.equals(env, "10.0.0.10C504", str(xcode_version_info.xcode_version()))
     asserts.equals(env, "local", xcode_version_info.availability())
     asserts.true(env, "requires-darwin" in xcode_version_info.execution_info())
     asserts.true(env, "no-remote" in xcode_version_info.execution_info())
     asserts.true(env, "supports-xcode-requirements-set" in xcode_version_info.execution_info())
 
+    assert_warning(
+        env,
+        "local_default_not_available_remotely",
+        "local_version=10.0.0.10C504;remote_versions=10.0",
+    )
+
     return analysistest.end(env)
 
 _prefer_local_default_if_no_mutual_no_flag_different_build_alias_test = analysistest.make(
@@ -505,16 +524,18 @@ def _prefer_local_default_if_no_mutual_no_flag_different_full_version_test_impl(
     target_under_test = analysistest.target_under_test(env)
     xcode_version_info = target_under_test[apple_common.XcodeVersionConfig]
 
-    # TODO: b/311385128 - Once we move the rules to apple_support, hack up
-    # something that would let us actually test the warning messages. We can't
-    # test `print`.
-
     asserts.equals(env, "10.0.0.10C504", str(xcode_version_info.xcode_version()))
     asserts.equals(env, "local", xcode_version_info.availability())
     asserts.true(env, "requires-darwin" in xcode_version_info.execution_info())
     asserts.true(env, "no-remote" in xcode_version_info.execution_info())
     asserts.true(env, "supports-xcode-requirements-set" in xcode_version_info.execution_info())
 
+    assert_warning(
+        env,
+        "local_default_not_available_remotely",
+        "local_version=10.0.0.10C504;remote_versions=10.0.0.101ff",
+    )
+
     return analysistest.end(env)
 
 _prefer_local_default_if_no_mutual_no_flag_different_full_version_test = analysistest.make(
@@ -554,10 +575,6 @@ def _choose_newest_mutual_xcode_test_impl(ctx):
     target_under_test = analysistest.target_under_test(env)
     xcode_version_info = target_under_test[apple_common.XcodeVersionConfig]
 
-    # TODO: b/311385128 - Once we move the rules to apple_support, hack up
-    # something that would let us actually test the warning messages. We can't
-    # test `print`.
-
     asserts.equals(env, "10", str(xcode_version_info.xcode_version()))
     asserts.equals(env, "both", xcode_version_info.availability())
     asserts.true(env, "requires-darwin" in xcode_version_info.execution_info())
diff --git a/xcode/xcode_config.bzl b/xcode/xcode_config.bzl
@@ -23,7 +23,7 @@ load(
 
 visibility("public")
 
-unavailable_xcode_message = "'bazel fetch --configure' (Bzlmod) or 'bazel sync --configure' (WORKSPACE)"
+UNAVAILABLE_XCODE_MESSAGE = "'bazel fetch --configure' (Bzlmod) or 'bazel sync --configure' (WORKSPACE)"
 
 def _xcode_config_impl(ctx):
     apple_fragment = ctx.fragments.apple
@@ -54,6 +54,7 @@ def _xcode_config_impl(ctx):
         remote_versions,
     ):
         xcode_version_properties, availability = _resolve_xcode_from_local_and_remote(
+            ctx.actions,
             local_versions,
             remote_versions,
             apple_fragment.xcode_version_flag,
@@ -208,6 +209,7 @@ def _aliases_to_xcode_version(versions):
     return version_map
 
 def _resolve_xcode_from_local_and_remote(
+        actions,
         local_versions,
         remote_versions,
         xcode_version_flag,
@@ -256,32 +258,29 @@ def _resolve_xcode_from_local_and_remote(
                 )
 
         elif local_version_from_flag:
-            error = (
-                " --xcode_version={} specified, but it is not available remotely. Actions " +
-                "requiring Xcode will be run locally, which could make your build slower."
-            ).format(
-                xcode_version_flag,
-            )
-            if (mutually_available_versions):
-                error += " Consider using one of [{}].".format(
-                    ", ".join([version for version in mutually_available_versions]),
+            if mutually_available_versions:
+                _warn(
+                    actions,
+                    "explicit_version_not_available_remotely_consider_mutual",
+                    version = xcode_version_flag,
+                    mutual_versions = [version for version in mutually_available_versions],
+                )
+            else:
+                _warn(
+                    actions,
+                    "explicit_version_not_available_remotely",
+                    version = xcode_version_flag,
                 )
-
-            # buildifier: disable=print
-            print(error)
             return local_version_from_flag.xcode_version_properties, "LOCAL"
 
         elif remote_version_from_flag:
-            # buildifier: disable=print
-            print(("--xcode_version={version} specified, but it is not available locally. " +
-                   "Your build will fail if any actions require a local Xcode. " +
-                   "If you believe you have '{version}' installed, try running {command}," +
-                   "and then re-run your command. Locally available versions: {local_versions}. ")
-                .format(
+            _warn(
+                actions,
+                "version_not_available_locally",
                 version = xcode_version_flag,
-                command = unavailable_xcode_message,
+                command = UNAVAILABLE_XCODE_MESSAGE,
                 local_versions = ", ".join([version for version in local_alias_to_version_map.keys()]),
-            ))
+            )
             availability = "REMOTE"
 
             return remote_version_from_flag.xcode_version_properties, availability
@@ -293,7 +292,7 @@ def _resolve_xcode_from_local_and_remote(
                  " you believe you have '{0}' installed, try running {1}, and then" +
                  " re-run your command.").format(
                     xcode_version_flag,
-                    unavailable_xcode_message,
+                    UNAVAILABLE_XCODE_MESSAGE,
                     ", ".join([version.xcode_version_properties.xcode_version for version in local_versions]),
                     ", ".join([version.xcode_version_properties.xcode_version for version in remote_versions]),
                 ),
@@ -305,12 +304,11 @@ def _resolve_xcode_from_local_and_remote(
 
     # If there aren't any mutually available versions, select the local default.
     if not mutually_available_versions:
-        # buildifier: disable=print
-        print(
-            ("Using a local Xcode version, '{}', since there are no" +
-             " remotely available Xcodes on this machine. Consider downloading one of the" +
-             " remotely available Xcode versions ({}) in order to get the best build" +
-             " performance.").format(local_default_version.xcode_version_properties.xcode_version, ", ".join([version.xcode_version_properties.xcode_version for version in remote_versions])),
+        _warn(
+            actions,
+            "local_default_not_available_remotely",
+            local_version = local_default_version.xcode_version_properties.xcode_version,
+            remote_versions = ", ".join([version.xcode_version_properties.xcode_version for version in remote_versions]),
         )
         local_version = local_default_version
         availability = "LOCAL"
@@ -385,3 +383,57 @@ def _resolve_explicitly_defined_version(
 
 def _dotted_version_or_default(field, default):
     return apple_common.dotted_version(field) or default
+
+_WARNINGS = {
+    "version_not_available_locally": """\
+--xcode_version={version} specified, but it is not available locally. \
+Your build will fail if any actions require a local Xcode. \
+If you believe you have '{version}' installed, try running {command}, \
+and then re-run your command. Locally available versions: {local_versions}.
+""",
+    "local_default_not_available_remotely": """\
+Using a local Xcode version, '{local_version}', since there are no \
+remotely available Xcodes on this machine. Consider downloading one of the \
+remotely available Xcode versions ({remote_versions}) in order to get the best \
+build performance.
+""",
+    "explicit_version_not_available_remotely": """\
+--xcode_version={version} specified, but it is not available remotely. Actions \
+requiring Xcode will be run locally, which could make your build slower.
+""",
+    "explicit_version_not_available_remotely_consider_mutual": """\
+--xcode_version={version} specified, but it is not available remotely. Actions \
+requiring Xcode will be run locally, which could make your build slower. \
+Consider using one of [{mutual_versions}].
+""",
+}
+
+def _warn(actions, msg_id, **kwargs):
+    """Print a warning and also record it as a testable dummy action.
+
+    Starlark doesn't support testing the output of the `print()` function, so
+    this function also registers a dummy action with a specifically formatted
+    mnemonic that can be read in the test using the `assert_warning` helper.
+
+    Args:
+        actions: The object used to register actions.
+        msg_id: A string identifying the warning as a key in the `_WARNINGS`
+            dictionary.
+        **kwargs: Formatting arguments for the message string.
+    """
+
+    # buildifier: disable=print
+    print(_WARNINGS[msg_id].format(**kwargs))
+
+    mnemonic = "Warning:{}".format(msg_id)
+    if kwargs:
+        # Sort the format arguments by key so that they're deterministically
+        # ordered for tests.
+        sorted_values = []
+        for key in sorted(kwargs.keys()):
+            sorted_values.append("{}={}".format(key, str(kwargs[key])))
+        mnemonic += ":{}".format(";".join(sorted_values))
+
+    actions.do_nothing(
+        mnemonic = mnemonic,
+    )
