fix(config): ensure the actually used config file is correct, better test coverage

Author: bearomorphism

commitizen/config/__init__.py

@@ -1,62 +1,64 @@
 from __future__ import annotations
 
 from pathlib import Path
-from typing import TYPE_CHECKING
 
 from commitizen import defaults, git, out
 from commitizen.config.factory import create_config
 from commitizen.exceptions import ConfigFileIsEmpty, ConfigFileNotFound
 
 from .base_config import BaseConfig
 
-if TYPE_CHECKING:
-    from collections.abc import Generator
-
-
-def _resolve_config_paths(filepath: str | None = None) -> Generator[Path, None, None]:
-    if filepath is not None:
-        out_path = Path(filepath)
-        if not out_path.exists():
-            raise ConfigFileNotFound()
-
-        yield out_path
-        return
 
+def _resolve_config_paths() -> list[Path]:
     git_project_root = git.find_git_project_root()
     cfg_search_paths = [Path(".")]
-    if git_project_root:
+
+    if git_project_root and not cfg_search_paths[0].samefile(git_project_root):
         cfg_search_paths.append(git_project_root)
 
-    for path in cfg_search_paths:
+    # The following algorithm is ugly, but we need to ensure that the order of the candidates are preserved before v5.
+    # Also, the number of possible config files is limited, so the complexity is not a problem.
+    candidates: list[Path] = []
+    for dir in cfg_search_paths:
         for filename in defaults.CONFIG_FILES:
-            out_path = path / Path(filename)
-            if out_path.exists():
-                yield out_path
+            out_path = dir / Path(filename)
+            if out_path.exists() and all(not out_path.samefile(p) for p in candidates):
+                candidates.append(out_path)
+    return candidates
+
+
+def _create_config_from_path(path: Path) -> BaseConfig:
+    with open(path, "rb") as f:
+        data: bytes = f.read()
+
+    return create_config(data=data, path=path)
 
 
 def read_cfg(filepath: str | None = None) -> BaseConfig:
-    config_candidates = list(_resolve_config_paths(filepath))
+    if filepath is not None:
+        conf_path = Path(filepath)
+        if not conf_path.exists():
+            raise ConfigFileNotFound()
+        conf = _create_config_from_path(conf_path)
+        if conf.is_empty_config:
+            raise ConfigFileIsEmpty()
+        return conf
+
+    config_candidate_paths = _resolve_config_paths()
 
     # Check for multiple config files and warn the user
     config_candidates_exclude_pyproject = [
-        path for path in config_candidates if path.name != "pyproject.toml"
+        path for path in config_candidate_paths if path.name != "pyproject.toml"
     ]
-    if len(config_candidates_exclude_pyproject) > 1:
-        filenames = [path.name for path in config_candidates_exclude_pyproject]
-        out.warn(
-            f"Multiple config files detected: {', '.join(filenames)}. "
-            f"Using config file: '{filenames[0]}'."
-        )
-
-    for filename in config_candidates:
-        with open(filename, "rb") as f:
-            data: bytes = f.read()
-
-        conf = create_config(data=data, path=filename)
+
+    for config_candidate_path in config_candidate_paths:
+        conf = _create_config_from_path(config_candidate_path)
         if not conf.is_empty_config:
+            if len(config_candidates_exclude_pyproject) > 1:
+                out.warn(
+                    f"Multiple config files detected: {', '.join(map(str, config_candidates_exclude_pyproject))}. "
+                    f"Using config file: '{config_candidate_path}'."
+                )
             return conf
 
-        if filepath is not None:
-            raise ConfigFileIsEmpty()
-
     return BaseConfig()

tests/test_conf.py

@@ -8,7 +8,7 @@
 import pytest
 import yaml
 
-from commitizen import config, defaults, git
+from commitizen import cmd, config, defaults, git
 from commitizen.config.json_config import JsonConfig
 from commitizen.config.toml_config import TomlConfig
 from commitizen.config.yaml_config import YAMLConfig
@@ -239,71 +239,92 @@ def test_load_empty_pyproject_toml_from_config_argument(_, tmpdir):
             with pytest.raises(ConfigFileIsEmpty):
                 config.read_cfg(filepath="./not_in_root/pyproject.toml")
 
-    def test_warn_multiple_config_files(_, tmpdir, capsys):
-        """Test that a warning is issued when multiple config files exist."""
-        with tmpdir.as_cwd():
-            # Create multiple config files
-            tmpdir.join(".cz.toml").write(PYPROJECT)
-            tmpdir.join(".cz.json").write(JSON_STR)
-
-            # Read config
-            cfg = config.read_cfg()
 
-            # Check that the warning was issued
-            captured = capsys.readouterr()
-            assert "Multiple config files detected" in captured.err
-            assert ".cz.toml" in captured.err
-            assert ".cz.json" in captured.err
-            assert "Using" in captured.err
-
-            # Verify the correct config is loaded (first in priority order)
-            assert cfg.settings == _settings
-
-    def test_warn_multiple_config_files_with_pyproject(_, tmpdir, capsys):
-        """Test warning excludes pyproject.toml from the warning message."""
+class TestWarnMultipleConfigFiles:
+    @pytest.mark.parametrize(
+        "files,expected_path,should_warn",
+        [
+            # Same directory, different file types
+            ([(".cz.toml", PYPROJECT), (".cz.json", JSON_STR)], ".cz.toml", True),
+            ([(".cz.json", JSON_STR), (".cz.yaml", YAML_STR)], ".cz.json", True),
+            ([(".cz.toml", PYPROJECT), (".cz.yaml", YAML_STR)], ".cz.toml", True),
+            # With pyproject.toml (excluded from warning)
+            (
+                [("pyproject.toml", PYPROJECT), (".cz.json", JSON_STR)],
+                "pyproject.toml",
+                False,
+            ),
+            (
+                [("pyproject.toml", PYPROJECT), (".cz.toml", PYPROJECT)],
+                "pyproject.toml",
+                False,
+            ),
+        ],
+    )
+    def test_warn_multiple_config_files_same_dir(
+        _, tmpdir, capsys, files, expected_path, should_warn
+    ):
+        """Test warning when multiple config files exist in same directory."""
         with tmpdir.as_cwd():
-            # Create multiple config files including pyproject.toml
-            tmpdir.join("pyproject.toml").write(PYPROJECT)
-            tmpdir.join(".cz.json").write(JSON_STR)
+            for filename, content in files:
+                tmpdir.join(filename).write(content)
 
-            # Read config - should use pyproject.toml (first in priority)
             cfg = config.read_cfg()
-
-            # No warning should be issued as only one non-pyproject config exists
             captured = capsys.readouterr()
-            assert "Multiple config files detected" not in captured.err
 
-            # Verify the correct config is loaded
-            assert cfg.settings == _settings
+            if should_warn:
+                assert "Multiple config files detected" in captured.err
+                assert "Using" in captured.err
+                for filename, _ in files:
+                    if filename != "pyproject.toml":
+                        assert filename in captured.err
+            else:
+                assert "Multiple config files detected" not in captured.err
+
+            assert cfg.path == Path(expected_path)
+            # Verify config loaded correctly (name and version match expected)
+            assert cfg.settings["name"] == "cz_jira"
+            assert cfg.settings["version"] == "1.0.0"
 
-    def test_warn_multiple_config_files_uses_correct_one(_, tmpdir, capsys):
-        """Test that the correct config file is used when multiple exist."""
+    @pytest.mark.parametrize(
+        "config_file",
+        [".cz.json", ".cz.toml", ".cz.yaml"],
+    )
+    def test_warn_same_filename_different_directories(_, tmpdir, capsys, config_file):
+        """Test warning when same config filename exists in different directories."""
         with tmpdir.as_cwd():
-            # Create .cz.json with different settings
-            json_different = """
-            {
-                "commitizen": {
-                    "name": "cz_conventional_commits",
-                    "version": "2.0.0"
-                }
-            }
-            """
-            tmpdir.join(".cz.json").write(json_different)
-            tmpdir.join(".cz.toml").write(PYPROJECT)
-
-            # Read config - should use pyproject.toml (first in defaults.CONFIG_FILES)
-            # But since pyproject.toml doesn't exist, .cz.toml is second in priority
-            cfg = config.read_cfg()
-
-            # Check that warning mentions both files
-            captured = capsys.readouterr()
-            assert "Multiple config files detected" in captured.err
-            assert ".cz.toml" in captured.err
-            assert ".cz.json" in captured.err
-
-            # Verify .cz.toml was used (second in priority after pyproject.toml)
-            assert cfg.settings["name"] == "cz_jira"  # from PYPROJECT
-            assert cfg.settings["version"] == "1.0.0"
+            cmd.run("git init")
+
+            # Create config in git root
+            if config_file.endswith(".json"):
+                root_content = '{"commitizen": {"name": "cz_jira", "version": "1.0.0"}}'
+            elif config_file.endswith(".yaml"):
+                root_content = "commitizen:\n  name: cz_jira\n  version: 1.0.0"
+            else:
+                root_content = PYPROJECT
+            tmpdir.join(config_file).write(root_content)
+
+            # Create same filename in subdirectory
+            subdir = tmpdir.mkdir("subdir")
+            if config_file.endswith(".json"):
+                subdir_content = '{"commitizen": {"name": "cz_conventional_commits", "version": "2.0.0"}}'
+            elif config_file.endswith(".yaml"):
+                subdir_content = (
+                    "commitizen:\n  name: cz_conventional_commits\n  version: 2.0.0"
+                )
+            else:
+                subdir_content = PYPROJECT.replace(
+                    'name = "cz_jira"', 'name = "cz_conventional_commits"'
+                ).replace('version = "1.0.0"', 'version = "2.0.0"')
+            subdir.join(config_file).write(subdir_content)
+
+            with subdir.as_cwd():
+                cfg = config.read_cfg()
+                captured = capsys.readouterr()
+
+                assert "Multiple config files detected" in captured.err
+                assert f"Using config file: '{config_file}'" in captured.err
+                assert cfg.path == Path(config_file)
 
     def test_no_warn_with_explicit_config_path(_, tmpdir, capsys):
         """Test that no warning is issued when user explicitly specifies config."""
@@ -323,6 +344,44 @@ def test_no_warn_with_explicit_config_path(_, tmpdir, capsys):
             json_cfg_expected = JsonConfig(data=JSON_STR, path=Path(".cz.json"))
             assert cfg.settings == json_cfg_expected.settings
 
+    @pytest.mark.parametrize(
+        "config_file, content, with_git",
+        [
+            (".cz.toml", PYPROJECT, True),
+            (".cz.json", JSON_STR, True),
+            (".cz.yaml", YAML_STR, True),
+            ("pyproject.toml", PYPROJECT, True),
+            ("cz.toml", PYPROJECT, True),
+            ("cz.json", JSON_STR, True),
+            ("cz.yaml", YAML_STR, True),
+            (".cz.toml", PYPROJECT, False),
+            (".cz.json", JSON_STR, False),
+            (".cz.yaml", YAML_STR, False),
+            ("pyproject.toml", PYPROJECT, False),
+            ("cz.toml", PYPROJECT, False),
+            ("cz.json", JSON_STR, False),
+            ("cz.yaml", YAML_STR, False),
+        ],
+    )
+    def test_no_warn_with_single_config_file(
+        _, tmpdir, capsys, config_file, content, with_git
+    ):
+        """Test that no warning is issued when user explicitly specifies config."""
+        with tmpdir.as_cwd():
+            if with_git:
+                cmd.run("git init")
+            # Create multiple config files
+            tmpdir.join(config_file).write(content)
+
+            # Read config with explicit path
+            cfg = config.read_cfg()
+            captured = capsys.readouterr()
+
+            # No warning should be issued
+            assert "Multiple config files detected" not in captured.err
+
+            assert cfg.path == Path(config_file)
+
 
 @pytest.mark.parametrize(
     "config_file, exception_string",