Add tests

python/private/pypi/simpleapi_download.bzl

@@ -23,7 +23,81 @@ load("//python/private:normalize_name.bzl", "normalize_name")
 load("//python/private:text_util.bzl", "render")
 load(":parse_simpleapi_html.bzl", "parse_simpleapi_html")
 
-def simpleapi_download(ctx, *, attr, cache, parallel_download = True):
+def _read_simpleapi(ctx, url, attr, cache, **download_kwargs):
+    """Read SimpleAPI.
+
+    Args:
+        ctx: The module_ctx or repository_ctx.
+        url: str, the url parameter that can be passed to ctx.download.
+        attr: The attribute that contains necessary info for downloading. The
+          following attributes must be present:
+           * envsubst: The envsubst values for performing substitutions in the URL.
+           * netrc: The netrc parameter for ctx.download, see http_file for docs.
+           * auth_patterns: The auth_patterns parameter for ctx.download, see
+               http_file for docs.
+        cache: A dict for storing the results.
+        **download_kwargs: Any extra params to ctx.download.
+            Note that output and auth will be passed for you.
+
+    Returns:
+        A similar object to what `download` would return except that in result.out
+        will be the parsed simple api contents.
+    """
+    # NOTE @aignas 2024-03-31: some of the simple APIs use relative URLs for
+    # the whl location and we cannot handle multiple URLs at once by passing
+    # them to ctx.download if we want to correctly handle the relative URLs.
+    # TODO: Add a test that env subbed index urls do not leak into the lock file.
+
+    real_url = envsubst(
+        url,
+        attr.envsubst,
+        ctx.getenv if hasattr(ctx, "getenv") else ctx.os.environ.get,
+    )
+
+    cache_key = real_url
+    if cache_key in cache:
+        return struct(success = True, output = cache[cache_key])
+
+    output_str = envsubst(
+        url,
+        attr.envsubst,
+        # Use env names in the subst values - this will be unique over
+        # the lifetime of the execution of this function and we also use
+        # `~` as the separator to ensure that we don't get clashes.
+        {e: "~{}~".format(e) for e in attr.envsubst}.get,
+    )
+
+    # Transform the URL into a valid filename
+    for char in [".", ":", "/", "\\", "-"]:
+        output_str = output_str.replace(char, "_")
+
+    output = ctx.path(output_str.strip("_").lower() + ".html")
+
+    # NOTE: this may have block = True or block = False in the download_kwargs
+    download = ctx.download(
+        url = [real_url],
+        output = output,
+        auth = get_auth(ctx, [real_url], ctx_attr = attr),
+        allow_fail = True,
+        **download_kwargs
+    )
+
+    if download_kwargs.get("block") == False:
+        # Simulate the same API as ctx.download has
+        return struct(
+            wait = lambda: _read_index_result(ctx, download.wait(), output, real_url, cache, cache_key),
+        )
+
+    return _read_index_result(ctx, download, output, real_url, cache, cache_key)
+
+def simpleapi_download(
+        ctx,
+        *,
+        attr,
+        cache,
+        parallel_download = True,
+        read_simpleapi = _read_simpleapi,
+        _fail = fail):
     """Download Simple API HTML.
 
     Args:
@@ -50,6 +124,9 @@ def simpleapi_download(ctx, *, attr, cache, parallel_download = True):
             reflected when re-evaluating the extension unless we do
             `bazel clean --expunge`.
         parallel_download: A boolean to enable usage of bazel 7.1 non-blocking downloads.
+        read_simpleapi: a function for reading and parsing of the SimpleAPI contents.
+            Used in tests.
+        _fail: a function to print a failure. Used in tests.
 
     Returns:
         dict of pkg name to the parsed HTML contents - a list of structs.
@@ -79,7 +156,7 @@ def simpleapi_download(ctx, *, attr, cache, parallel_download = True):
         sources = [pkg for pkg in attr.sources if pkg not in found_on_index]
         for pkg in sources:
             pkg_normalized = normalize_name(pkg)
-            result = _read_simpleapi(
+            result = read_simpleapi(
                 ctx = ctx,
                 url = "{}/{}/".format(
                     index_url_overrides.get(pkg_normalized, index_url).rstrip("/"),
@@ -95,7 +172,7 @@ def simpleapi_download(ctx, *, attr, cache, parallel_download = True):
                     pkg_normalized = pkg_normalized,
                     wait = result.wait,
                 )
-            else:
+            elif result.success:
                 contents[pkg_normalized] = result.output
                 found_on_index[pkg] = index_url
 
@@ -113,10 +190,11 @@ def simpleapi_download(ctx, *, attr, cache, parallel_download = True):
 
     failed_sources = [pkg for pkg in attr.sources if pkg not in found_on_index]
     if failed_sources:
-        fail("Failed to download metadata for {} for from urls: {}".format(
+        _fail("Failed to download metadata for {} for from urls: {}".format(
             failed_sources,
             index_urls,
         ))
+        return None
 
     if warn_overrides:
         index_url_overrides = {
@@ -132,73 +210,6 @@ def simpleapi_download(ctx, *, attr, cache, parallel_download = True):
 
     return contents
 
-def _read_simpleapi(ctx, url, attr, cache, **download_kwargs):
-    """Read SimpleAPI.
-
-    Args:
-        ctx: The module_ctx or repository_ctx.
-        url: str, the url parameter that can be passed to ctx.download.
-        attr: The attribute that contains necessary info for downloading. The
-          following attributes must be present:
-           * envsubst: The envsubst values for performing substitutions in the URL.
-           * netrc: The netrc parameter for ctx.download, see http_file for docs.
-           * auth_patterns: The auth_patterns parameter for ctx.download, see
-               http_file for docs.
-        cache: A dict for storing the results.
-        **download_kwargs: Any extra params to ctx.download.
-            Note that output and auth will be passed for you.
-
-    Returns:
-        A similar object to what `download` would return except that in result.out
-        will be the parsed simple api contents.
-    """
-    # NOTE @aignas 2024-03-31: some of the simple APIs use relative URLs for
-    # the whl location and we cannot handle multiple URLs at once by passing
-    # them to ctx.download if we want to correctly handle the relative URLs.
-    # TODO: Add a test that env subbed index urls do not leak into the lock file.
-
-    real_url = envsubst(
-        url,
-        attr.envsubst,
-        ctx.getenv if hasattr(ctx, "getenv") else ctx.os.environ.get,
-    )
-
-    cache_key = real_url
-    if cache_key in cache:
-        return struct(success = True, output = cache[cache_key])
-
-    output_str = envsubst(
-        url,
-        attr.envsubst,
-        # Use env names in the subst values - this will be unique over
-        # the lifetime of the execution of this function and we also use
-        # `~` as the separator to ensure that we don't get clashes.
-        {e: "~{}~".format(e) for e in attr.envsubst}.get,
-    )
-
-    # Transform the URL into a valid filename
-    for char in [".", ":", "/", "\\", "-"]:
-        output_str = output_str.replace(char, "_")
-
-    output = ctx.path(output_str.strip("_").lower() + ".html")
-
-    # NOTE: this may have block = True or block = False in the download_kwargs
-    download = ctx.download(
-        url = [real_url],
-        output = output,
-        auth = get_auth(ctx, [real_url], ctx_attr = attr),
-        allow_fail = True,
-        **download_kwargs
-    )
-
-    if download_kwargs.get("block") == False:
-        # Simulate the same API as ctx.download has
-        return struct(
-            wait = lambda: _read_index_result(ctx, download.wait(), output, real_url, cache, cache_key),
-        )
-
-    return _read_index_result(ctx, download, output, real_url, cache, cache_key)
-
 def _read_index_result(ctx, result, output, url, cache, cache_key):
     if not result.success:
         return struct(success = False)

tests/pypi/simpleapi_download/BUILD.bazel

@@ -0,0 +1,5 @@
+load("simpleapi_download_tests.bzl", "simpleapi_download_test_suite")
+
+simpleapi_download_test_suite(
+    name = "simpleapi_download_tests",
+)

tests/pypi/simpleapi_download/simpleapi_download_tests.bzl

@@ -0,0 +1,128 @@
+# Copyright 2024 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+""
+
+load("@rules_testing//lib:test_suite.bzl", "test_suite")
+load("//python/private/pypi:simpleapi_download.bzl", "simpleapi_download")  # buildifier: disable=bzl-visibility
+
+_tests = []
+
+def _test_simple(env):
+    calls = []
+
+    def read_simpleapi(ctx, url, attr, cache, block):
+        _ = ctx  # buildifier: disable=unused-variable
+        _ = attr
+        _ = cache
+        env.expect.that_bool(block).equals(False)
+        calls.append(url)
+        if "foo" in url and "main" in url:
+            return struct(
+                output = "",
+                success = False,
+            )
+        else:
+            return struct(
+                output = "data from {}".format(url),
+                success = True,
+            )
+
+    contents = simpleapi_download(
+        ctx = struct(
+            os = struct(environ = {}),
+        ),
+        attr = struct(
+            index_url_overrides = {},
+            index_url = "main",
+            extra_index_urls = ["extra"],
+            sources = ["foo", "bar", "baz"],
+            envsubst = [],
+        ),
+        cache = {},
+        parallel_download = True,
+        read_simpleapi = read_simpleapi,
+    )
+
+    env.expect.that_collection(calls).contains_exactly([
+        "extra/foo/",
+        "main/bar/",
+        "main/baz/",
+        "main/foo/",
+    ])
+    env.expect.that_dict(contents).contains_exactly({
+        "bar": "data from main/bar/",
+        "baz": "data from main/baz/",
+        "foo": "data from extra/foo/",
+    })
+
+_tests.append(_test_simple)
+
+def _test_fail(env):
+    calls = []
+    fails = []
+
+    def read_simpleapi(ctx, url, attr, cache, block):
+        _ = ctx  # buildifier: disable=unused-variable
+        _ = attr
+        _ = cache
+        env.expect.that_bool(block).equals(False)
+        calls.append(url)
+        if "foo" in url:
+            return struct(
+                output = "",
+                success = False,
+            )
+        else:
+            return struct(
+                output = "data from {}".format(url),
+                success = True,
+            )
+
+    simpleapi_download(
+        ctx = struct(
+            os = struct(environ = {}),
+        ),
+        attr = struct(
+            index_url_overrides = {},
+            index_url = "main",
+            extra_index_urls = ["extra"],
+            sources = ["foo", "bar", "baz"],
+            envsubst = [],
+        ),
+        cache = {},
+        parallel_download = True,
+        read_simpleapi = read_simpleapi,
+        _fail = fails.append,
+    )
+
+    env.expect.that_collection(fails).contains_exactly([
+        """Failed to download metadata for ["foo"] for from urls: ["main", "extra"]""",
+    ])
+    env.expect.that_collection(calls).contains_exactly([
+        "extra/foo/",
+        "main/bar/",
+        "main/baz/",
+        "main/foo/",
+    ])
+
+_tests.append(_test_fail)
+
+def simpleapi_download_test_suite(name):
+    """Create the test suite.
+
+    Args:
+        name: the name of the test suite
+    """
+    test_suite(name = name, basic_tests = _tests)