From fa5a40a8cae0c12e8f1debe00c68d78872ab8563 Mon Sep 17 00:00:00 2001 From: Yu-Ting Hsiung Date: Mon, 12 Jan 2026 17:23:16 +0800 Subject: [PATCH] fix(config): ensure the actually used config file is correct, better test coverage --- commitizen/config/__init__.py | 53 ++++++------- tests/test_conf.py | 137 ++++++++++++++++++++-------------- 2 files changed, 106 insertions(+), 84 deletions(-) diff --git a/commitizen/config/__init__.py b/commitizen/config/__init__.py index 2fb84a123..02f8a9e6a 100644 --- a/commitizen/config/__init__.py +++ b/commitizen/config/__init__.py @@ -13,15 +13,7 @@ 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() -> Generator[Path, None, None]: git_project_root = git.find_git_project_root() cfg_search_paths = [Path(".")] if git_project_root: @@ -34,29 +26,38 @@ def _resolve_config_paths(filepath: str | None = None) -> Generator[Path, None, yield out_path +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: + out_path = Path(filepath) + if not out_path.exists(): + raise ConfigFileNotFound() + conf = _create_config_from_path(out_path) + if conf.is_empty_config: + raise ConfigFileIsEmpty() + return conf + + config_candidate_paths = list(_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() diff --git a/tests/test_conf.py b/tests/test_conf.py index 1ef30d515..366dbad20 100644 --- a/tests/test_conf.py +++ b/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."""