test(ENG-13681): fold duplicated cases together

The suite had grown several tests per behaviour: three separate
fallback-to-builtins tests, four on one lookup failure, two each on the
entry key set and the built-in entry fields, and a repository-scoping pair
that asserted what the precedence tests already prove.

Merged them into parametrized cases and dropped the redundant ones. Every
behaviour still has a test, including the failure paths; there is just one
test per behaviour now instead of three. Also removed the narrative comments
inside the tests, leaving the docstrings to say what each one covers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: cloudsmith-iduffy

cloudsmith_cli/cli/tests/commands/test_credential_helper.py

@@ -244,8 +244,7 @@ def _cache_dir(tmp_path, monkeypatch):
     [
         # 200 → records built and cached
         (200, True, True),
-        # 404 → [] but NOT cached (org not found - probably a typo, must
-        # fail loudly every time rather than replay a cached failure)
+        # 404 → [] but NOT cached
         (404, False, False),
         # 403 → [] but NOT cached (may succeed after auth)
         (403, False, False),
@@ -472,8 +471,6 @@ def test_get_custom_domains_bearer_credential_sends_authorization_header(
         content_type="application/json",
     )
 
-    # A previously configured API key must not leak into a bearer-authenticated
-    # request: initialise_api clears the class-default X-Api-Key when switching.
     initialise_api(
         host=API_HOST,
         credential=CredentialResult(api_key="k_stale", source_name="test"),
@@ -749,10 +746,7 @@ def test_is_cloudsmith_domain(
 @pytest.mark.parametrize(
     "credential",
     [
-        # no credential at all
         None,
-        # a credential carrying no usable key — truthy as an object, so the
-        # guard has to look at api_key, not just the credential
         CredentialResult(api_key="", source_name="test"),
     ],
 )
@@ -850,7 +844,6 @@ def test_custom_domain_scope(repository, expected):
     [
         ("repository", DomainScope.REPOSITORY),
         ("organization", DomainScope.ORGANIZATION),
-        # A hand-edited config must not break every wrapped run.
         ("nonsense", DomainScope.ORGANIZATION),
         (None, DomainScope.ORGANIZATION),
         ("", DomainScope.ORGANIZATION),

cloudsmith_cli/cli/tests/commands/test_credential_helper_install.py

@@ -429,7 +429,6 @@ def _fake_get_format_domains(*_a, **kwargs):
         )
         assert captured["cache_only"] is True, "a dry run must not query the API"
         assert not (docker_dir / "config.json").exists()
-        # The preview names every host the real install would write.
         assert any("docker.acme.com" in a and "would set" in a for a in actions)
 
     else:  # discovery_raises — graceful failure guard

cloudsmith_cli/cli/tests/commands/test_custom_domain_precedence.py

@@ -121,32 +121,6 @@ def test_inactive_domains_are_never_chosen(enabled, validated):
     assert select_custom_domain([inactive], domain_type=DomainType.DOWNLOAD) is None
 
 
-def test_naming_a_repository_is_enough_to_be_scoped_to_it():
-    """The repository alone binds the domain.
-
-    The server refuses to save a repository and its `repository_only` flag
-    apart, so the flag can only ever restate the repository.
-    """
-    repo_bound = _domain("dl-prod.acme.com", repository="prod")
-
-    chosen = select_custom_domain(
-        [repo_bound], domain_type=DomainType.DOWNLOAD, repository="prod"
-    )
-
-    assert chosen is repo_bound
-
-
-def test_a_domain_bound_elsewhere_is_never_chosen():
-    """A repository-scoped host can never stand in for another repository."""
-    bound_elsewhere = _domain("dl-staging.acme.com", repository="staging")
-
-    chosen = select_custom_domain(
-        [bound_elsewhere], domain_type=DomainType.DOWNLOAD, repository="prod"
-    )
-
-    assert chosen is None
-
-
 def test_a_domain_bound_to_another_repository_is_never_used():
     """A repository-scoped host serves only the repository it belongs to."""
     other = _repo_domain("dl-staging.acme.com", repository="staging")

cloudsmith_cli/cli/tests/commands/test_default_domains.py

@@ -66,11 +66,26 @@ def no_trusted_config(tmp_path, monkeypatch):
     monkeypatch.setattr(cli_config.ConfigReader, "config_searchpath", [str(empty)])
 
 
-def test_load_returns_builtins_when_no_config(tmp_path, no_trusted_config):
-    """With no config file present, the built-in table is used."""
-    assert load_default_domains(config_path=tmp_path / "absent.ini") == list(
-        BUILTIN_DOMAINS
-    )
+@pytest.mark.parametrize(
+    "body",
+    [
+        None,
+        "[default]\napi_host = https://api.cloudsmith.io\n",
+        "this is not valid ini [[[",
+    ],
+    ids=["absent", "no-domains-section", "malformed"],
+)
+def test_a_config_declaring_no_table_falls_back_to_builtins(
+    tmp_path, no_trusted_config, body
+):
+    """Only a readable [domains] section replaces the built-in table."""
+    config = tmp_path / "config.ini"
+    if body is None:
+        config = tmp_path / "absent.ini"
+    else:
+        config.write_text(body, encoding="utf-8")
+
+    assert load_default_domains(config_path=config) == list(BUILTIN_DOMAINS)
 
 
 def test_explicit_config_without_a_table_falls_back_to_a_trusted_one(
@@ -116,7 +131,6 @@ def test_config_section_replaces_builtins_wholesale(tmp_path, no_trusted_config)
     indexed = _by_host(domains)
     assert indexed["packages.internal.example.com"].backend_kind == (BackendKind.PYTHON)
     assert indexed["packages.internal.example.com"].format_label == "python"
-    # An empty value means "no single format".
     assert indexed["cdn.internal.example.com"].backend_kind is None
     assert indexed["cdn.internal.example.com"].format_label is None
     assert indexed["cdn.internal.example.com"].domain_type is DomainType.DOWNLOAD
@@ -172,26 +186,6 @@ def test_config_label_without_backend_kind_is_formatless(tmp_path, no_trusted_co
     assert domain.domain_type is DomainType.DOWNLOAD
 
 
-def test_config_without_domains_section_falls_back_to_builtins(
-    tmp_path, no_trusted_config
-):
-    """A config file that has no [domains] section changes nothing."""
-    config = tmp_path / "config.ini"
-    config.write_text(
-        "[default]\napi_host = https://api.cloudsmith.io\n", encoding="utf-8"
-    )
-
-    assert load_default_domains(config_path=config) == list(BUILTIN_DOMAINS)
-
-
-def test_unreadable_config_falls_back_to_builtins(tmp_path, no_trusted_config):
-    """A malformed config must not break the command."""
-    config = tmp_path / "config.ini"
-    config.write_text("this is not valid ini [[[", encoding="utf-8")
-
-    assert load_default_domains(config_path=config) == list(BUILTIN_DOMAINS)
-
-
 def test_trusted_lookup_skips_a_config_without_a_domains_section(tmp_path, monkeypatch):
     """The table comes from the first trusted config that declares one.
 
@@ -340,8 +334,6 @@ def test_untrusted_cwd_config_is_not_honoured(tmp_path, monkeypatch):
     a Cloudsmith host and harvest a token.
     """
     monkeypatch.chdir(tmp_path)
-    # Pin the trusted lookup to "nothing found" so the developer's real
-    # config.ini cannot influence what this test observes.
     monkeypatch.setattr(
         "cloudsmith_cli.credential_helpers.default_domains._trusted_domains",
         lambda: None,
@@ -467,14 +459,13 @@ def test_builtin_domains_with_a_backend_kind_are_native_api():
             assert domain.domain_type is DomainType.NATIVE_API, domain.host
 
 
-def test_builtin_host_for_type_resolves_download_and_upload():
-    """The download/upload hosts are looked up by type, not by constant."""
+def test_builtin_host_for_type_resolves_the_format_less_hosts():
+    """The download and upload hosts are looked up by type, not by constant.
+
+    NATIVE_API covers many hosts, so there is no single one to return.
+    """
     assert builtin_host_for_type(DomainType.DOWNLOAD) == "dl.cloudsmith.io"
     assert builtin_host_for_type(DomainType.UPLOAD) == "upload.cloudsmith.io"
-
-
-def test_builtin_host_for_type_rejects_ambiguous_native_api():
-    """NATIVE_API covers many hosts, so there is no single one to return."""
     with pytest.raises(ValueError):
         builtin_host_for_type(DomainType.NATIVE_API)
 
@@ -483,25 +474,20 @@ def test_builtin_host_for_type_rejects_ambiguous_native_api():
     "backend_kind,expected",
     [
         (BackendKind.MAVEN, DomainType.NATIVE_API),
-        # DEB is 0 and therefore falsy: a truthiness check here would
-        # misclassify it as a download host.
         (BackendKind.DEB, DomainType.NATIVE_API),
         (None, DomainType.DOWNLOAD),
     ],
+    ids=["maven", "deb-is-zero-so-falsy", "no-kind"],
 )
 def test_domain_type_for_backend_kind(backend_kind, expected):
     """A kind implies native API; its absence implies a download host."""
     assert domain_type_for_backend_kind(backend_kind) is expected
 
 
-def test_builtin_host_resolves_backend_kind():
-    """builtin_host maps a backend kind to its built-in service host."""
+def test_builtin_host_resolves_a_backend_kind_to_its_own_host():
+    """Formats served only via the CDN have no dedicated built-in host."""
     assert builtin_host(BackendKind.MAVEN) == "maven.cloudsmith.io"
     assert builtin_host(BackendKind.PYTHON) == "python.cloudsmith.io"
-
-
-def test_builtin_host_rejects_kind_without_dedicated_host():
-    """Formats served only via the CDN have no dedicated built-in host."""
     with pytest.raises(ValueError):
         builtin_host(BackendKind.DEB)