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

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()

commitizen/defaults.py

@@ -67,15 +67,15 @@ class Settings(TypedDict, total=False):
     breaking_change_exclamation_in_title: bool
 
 
-CONFIG_FILES: list[str] = [
-    "pyproject.toml",
+CONFIG_FILES: tuple[str, ...] = (
     ".cz.toml",
+    "cz.toml",
     ".cz.json",
     "cz.json",
     ".cz.yaml",
     "cz.yaml",
-    "cz.toml",
-]
+    "pyproject.toml",
+)
 ENCODING = "utf-8"
 
 DEFAULT_SETTINGS: Settings = {

docs/config/configuration_file.md

@@ -10,13 +10,15 @@ It is recommended to create a configuration file via our [`cz init`](../commands
 
 Configuration files are typically located in the root of your project directory. Commitizen searches for configuration files in the following order:
 
-1. `pyproject.toml` (in the `[tool.commitizen]` section)
-2. `.cz.toml`
+<!-- DEPENDENCY: commitizen/defaults.py CONFIG_FILES -->
+
+1. `.cz.toml`
+2. `cz.toml`
 3. `.cz.json`
 4. `cz.json`
 5. `.cz.yaml`
 6. `cz.yaml`
-7. `cz.toml`
+7. `pyproject.toml` (in the `[tool.commitizen]` section)
 
 The first valid configuration file found will be used. If no configuration file is found, Commitizen will use its default settings.
 
@@ -29,7 +31,7 @@ The first valid configuration file found will be used. If no configuration file
     ```
 
 !!! tip
-    For Python projects, it's recommended to add your Commitizen configuration to `pyproject.toml` to keep all project configuration in one place.
+    For Python projects, you can add your Commitizen configuration to `pyproject.toml` to keep all project configuration in one place.
 
 !!! warning "Multiple Configuration Files"
     If Commitizen detects more than one configuration file in your project directory (excluding `pyproject.toml`), it will display a warning message and identify which file is being used. To avoid confusion, ensure you have only one Commitizen configuration file in your project.

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
@@ -172,9 +172,7 @@ def test_find_git_project_root(tmpdir):
         assert git.find_git_project_root() is None
 
 
-@pytest.mark.parametrize(
-    "config_files_manager", defaults.CONFIG_FILES.copy(), indirect=True
-)
+@pytest.mark.parametrize("config_files_manager", defaults.CONFIG_FILES, indirect=True)
 def test_set_key(config_files_manager):
     _conf = config.read_cfg()
     _conf.set_key("version", "2.0.0")
@@ -184,7 +182,7 @@ def test_set_key(config_files_manager):
 
 class TestReadCfg:
     @pytest.mark.parametrize(
-        "config_files_manager", defaults.CONFIG_FILES.copy(), indirect=True
+        "config_files_manager", defaults.CONFIG_FILES, indirect=True
     )
     def test_load_conf(_, config_files_manager):
         cfg = config.read_cfg()
@@ -239,71 +237,85 @@ 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)],
+                ".cz.json",
+                False,
+            ),
+            (
+                [("pyproject.toml", PYPROJECT), (".cz.toml", PYPROJECT)],
+                ".cz.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,content",
+        [
+            (".cz.json", JSON_STR),
+            (".cz.toml", PYPROJECT),
+            (".cz.yaml", YAML_STR),
+            ("cz.toml", PYPROJECT),
+            ("cz.json", JSON_STR),
+            ("cz.yaml", YAML_STR),
+        ],
+    )
+    def test_warn_same_filename_different_directories_with_git(
+        _, tmpdir, capsys, config_file, content
+    ):
+        """Test warning when same config filename exists in the current directory and in the git root."""
         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)
+            cmd.run("git init")
 
-            # 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()
+            # Create config in git root
+            tmpdir.join(config_file).write(content)
 
-            # 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
+            # Create same filename in subdirectory
+            subdir = tmpdir.mkdir("subdir")
+            subdir.join(config_file).write(content)
 
-            # 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"
+            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 +335,39 @@ 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",
+        [
+            (file, content, with_git)
+            for file, content in [
+                (".cz.toml", PYPROJECT),
+                (".cz.json", JSON_STR),
+                (".cz.yaml", YAML_STR),
+                ("pyproject.toml", PYPROJECT),
+                ("cz.toml", PYPROJECT),
+                ("cz.json", JSON_STR),
+                ("cz.yaml", YAML_STR),
+            ]
+            for with_git in [True, 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")
+
+            tmpdir.join(config_file).write(content)
+
+            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",