Skip to content

test(pypi): add argparse.bzl tests #3697

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.

Sign up for GitHub

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
rickeylev merged 1 commit into from
Apr 12, 2026
+51 −0
Merged

test(pypi): add argparse.bzl tests #3697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions tests/pypi/argparse/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
load(":argparse_tests.bzl", "argparse_test_suite")

argparse_test_suite(name = "argparse_tests")
48 changes: 48 additions & 0 deletions tests/pypi/argparse/argparse_tests.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
""

load("@rules_testing//lib:test_suite.bzl", "test_suite")
load("//python/private/pypi:argparse.bzl", "argparse") # buildifier: disable=bzl-visibility

_tests = []

def _test_index_url(env):
env.expect.that_str(argparse.index_url([], "default")).equals("default")
env.expect.that_str(argparse.index_url([], None)).equals(None)

env.expect.that_str(argparse.index_url(["-i", "https://example.com/simple"], "default")).equals("https://example.com/simple")
env.expect.that_str(argparse.index_url(["--index-url", "https://example.com/simple"], "default")).equals("https://example.com/simple")
env.expect.that_str(argparse.index_url(["--index-url=https://example.com/simple"], "default")).equals("https://example.com/simple")

env.expect.that_str(argparse.index_url(["--extra-index-url", "https://extra.com", "-i", "https://index.com"], "default")).equals("https://index.com")

_tests.append(_test_index_url)
Comment on lines +8 to +18
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 12, 2026

Choose a reason for hiding this comment

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

medium

The test coverage for index_url can be improved by including cases for None as the args input and verifying that the last occurrence of the flag takes precedence when multiple are provided.

def _test_index_url(env):
    env.expect.that_str(argparse.index_url(None, "default")).equals("default")
    env.expect.that_str(argparse.index_url([], "default")).equals("default")
    env.expect.that_str(argparse.index_url([], None)).equals(None)

    env.expect.that_str(argparse.index_url(["-i", "https://example.com/simple"], "default")).equals("https://example.com/simple")
    env.expect.that_str(argparse.index_url(["--index-url", "https://example.com/simple"], "default")).equals("https://example.com/simple")
    env.expect.that_str(argparse.index_url(["--index-url=https://example.com/simple"], "default")).equals("https://example.com/simple")

    # Test that the last occurrence wins
    env.expect.that_str(argparse.index_url(["-i", "url1", "-i", "url2"], "default")).equals("url2")

    env.expect.that_str(argparse.index_url(["--extra-index-url", "https://extra.com", "-i", "https://index.com"], "default")).equals("https://index.com")

_tests.append(_test_index_url)


def _test_extra_index_url(env):
env.expect.that_collection(argparse.extra_index_url([], ["default"])).contains_exactly(["default"])
env.expect.that_collection(argparse.extra_index_url([], None)).contains_exactly([])

env.expect.that_collection(argparse.extra_index_url(["--extra-index-url", "https://extra.com/simple"], [])).contains_exactly(["https://extra.com/simple"])
env.expect.that_collection(argparse.extra_index_url(["--extra-index-url=https://extra.com/simple"], [])).contains_exactly(["https://extra.com/simple"])

env.expect.that_collection(argparse.extra_index_url(["--extra-index-url", "https://first.com", "--extra-index-url", "https://second.com"], [])).contains_exactly(["https://first.com", "https://second.com"])

_tests.append(_test_extra_index_url)
Comment on lines +20 to +29
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 12, 2026

Choose a reason for hiding this comment

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

medium

It is important to document and test the behavior when both a default value and command-line flags are provided for repeated arguments. The current implementation appends to the default list, which should be explicitly verified.

def _test_extra_index_url(env):
    env.expect.that_collection(argparse.extra_index_url([], ["default"])).contains_exactly(["default"])
    env.expect.that_collection(argparse.extra_index_url([], None)).contains_exactly([])

    env.expect.that_collection(argparse.extra_index_url(["--extra-index-url", "https://extra.com/simple"], [])).contains_exactly(["https://extra.com/simple"])
    env.expect.that_collection(argparse.extra_index_url(["--extra-index-url=https://extra.com/simple"], [])).contains_exactly(["https://extra.com/simple"])

    env.expect.that_collection(argparse.extra_index_url(["--extra-index-url", "https://first.com", "--extra-index-url", "https://second.com"], [])).contains_exactly(["https://first.com", "https://second.com"])

    # Test interaction between default and provided flags (currently appends)
    env.expect.that_collection(argparse.extra_index_url(["--extra-index-url", "url1"], ["default"])).contains_exactly(["default", "url1"])

_tests.append(_test_extra_index_url)


def _test_platform(env):
env.expect.that_collection(argparse.platform([], ["default"])).contains_exactly(["default"])
env.expect.that_collection(argparse.platform([], None)).contains_exactly([])

env.expect.that_collection(argparse.platform(["--platform", "manylinux_2_17_x86_64"], [])).contains_exactly(["manylinux_2_17_x86_64"])
env.expect.that_collection(argparse.platform(["--platform=manylinux_2_17_x86_64"], [])).contains_exactly(["manylinux_2_17_x86_64"])

env.expect.that_collection(argparse.platform(["--platform", "macosx_10_9_x86_64", "--platform", "linux_x86_64"], [])).contains_exactly(["macosx_10_9_x86_64", "linux_x86_64"])

_tests.append(_test_platform)
Comment on lines +31 to +40
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 12, 2026

Choose a reason for hiding this comment

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

medium

The implementation of _get_pip_args includes logic to deduplicate repeated flags. This behavior should be covered by a test case.

def _test_platform(env):
    env.expect.that_collection(argparse.platform([], ["default"])).contains_exactly(["default"])
    env.expect.that_collection(argparse.platform([], None)).contains_exactly([])

    env.expect.that_collection(argparse.platform(["--platform", "manylinux_2_17_x86_64"], [])).contains_exactly(["manylinux_2_17_x86_64"])
    env.expect.that_collection(argparse.platform(["--platform=manylinux_2_17_x86_64"], [])).contains_exactly(["manylinux_2_17_x86_64"])

    env.expect.that_collection(argparse.platform(["--platform", "macosx_10_9_x86_64", "--platform", "linux_x86_64"], [])).contains_exactly(["macosx_10_9_x86_64", "linux_x86_64"])

    # Test deduplication
    env.expect.that_collection(argparse.platform(["--platform", "p1", "--platform", "p1"], [])).contains_exactly(["p1"])

_tests.append(_test_platform)


def argparse_test_suite(name):
"""Create the test suite.

Args:
name: the name of the test suite
"""
test_suite(name = name, basic_tests = _tests)