diff --git a/py/envoy.code.check/envoy/code/check/abstract/changelog.py b/py/envoy.code.check/envoy/code/check/abstract/changelog.py index 88cf9d5f9b..0078bf9226 100644 --- a/py/envoy.code.check/envoy/code/check/abstract/changelog.py +++ b/py/envoy.code.check/envoy/code/check/abstract/changelog.py @@ -7,6 +7,8 @@ from collections.abc import Iterator from packaging import version as _version +from yamllint import linter as yamllint_linter # type:ignore +from yamllint.config import YamlLintConfig # type:ignore import abstracts @@ -19,9 +21,11 @@ ENTRY_SEPARATOR, ) from envoy.code.check import abstract, interface +from envoy.code.check.abstract.yamllint import YAMLLINT_CONFIG MAX_VERSION_FOR_CHANGES_SECTION = "1.16" +YAML_ENTRY_INDENT = " " VALID_CHANGELOG_AREA_RE = re.compile(r"^[a-z0-9_\-/]+$") VALID_CHANGELOG_AREA_PATTERN = r"[a-z0-9_\-/]+" @@ -35,9 +39,14 @@ class AChangelogChangesChecker(metaclass=abstracts.Abstraction): def __init__( self, sections: utils.typing.ChangelogSectionsDict, - areas: "utils.typing.ChangelogAreasDict") -> None: + areas: "utils.typing.ChangelogAreasDict", + *, + yamllint_config: YamlLintConfig | None = None, + yamllint_config_path: pathlib.Path | None = None) -> None: self.sections = sections self.areas = areas + self._yamllint_config = yamllint_config + self._yamllint_config_path = yamllint_config_path @property # type:ignore @abstracts.interfacemethod @@ -52,6 +61,17 @@ def config_path(self) -> pathlib.Path: def max_version_for_changes_section(self) -> _version.Version: return _version.Version(MAX_VERSION_FOR_CHANGES_SECTION) + @cached_property + def yamllint_config(self) -> YamlLintConfig: + if self._yamllint_config is not None: + return self._yamllint_config + config_path = ( + self._yamllint_config_path or pathlib.Path(YAMLLINT_CONFIG)) + return ( + YamlLintConfig(file=config_path) + if config_path.exists() + else YamlLintConfig("extends: default")) + def check_entry( self, version: _version.Version, @@ -174,6 +194,19 @@ def check_entry_content( f"{path}: Entry file is empty or contains only whitespace") return None + def check_entry_line_lengths( + self, + path: pathlib.Path) -> tuple[str, ...]: + content = "".join( + f"{YAML_ENTRY_INDENT}{line}\n" + for line + in path.read_text().splitlines()) + return tuple( + f"{path}:{problem.line}: {problem.desc}" + for problem + in yamllint_linter.run(content, self.yamllint_config) + if problem.rule == "line-length") + def check_entry_files( self, paths: list[pathlib.Path]) -> tuple[str, ...]: @@ -183,6 +216,7 @@ def check_entry_files( errors.append(err) if err := self.check_entry_content(path): errors.append(err) + errors.extend(self.check_entry_line_lengths(path)) return tuple(errors) @@ -384,7 +418,8 @@ def changes_checker_class( def changes_checker(self) -> interface.IChangelogChangesChecker: return self.changes_checker_class( self.project.changelogs.sections, - self.project.changelogs.areas) + self.project.changelogs.areas, + yamllint_config_path=self.project.path.joinpath(YAMLLINT_CONFIG)) @property # type:ignore @abstracts.interfacemethod diff --git a/py/envoy.code.check/tests/test_abstract_changelog.py b/py/envoy.code.check/tests/test_abstract_changelog.py index d09d9b19bd..bdd992e8df 100644 --- a/py/envoy.code.check/tests/test_abstract_changelog.py +++ b/py/envoy.code.check/tests/test_abstract_changelog.py @@ -4,6 +4,7 @@ from packaging import version as _version import pytest +from yamllint.config import YamlLintConfig from envoy.base import utils from envoy.code import check @@ -83,7 +84,9 @@ def test_changelogcheck_changes_checker(patches): assert ( m_class.return_value.call_args == [(project.changelogs.sections, - project.changelogs.areas), {}]) + project.changelogs.areas), + dict(yamllint_config_path=project.path.joinpath(".yamllint"))]) + assert project.path.joinpath.call_args == [(".yamllint", ), {}] assert "changes_checker" in changelog.__dict__ @@ -701,8 +704,8 @@ async def test_changelogstatus_check_areas_file( class DummyChangelogChangesChecker(check.AChangelogChangesChecker): - def __init__(self, sections, areas=None): - super().__init__(sections, {} if areas is None else areas) + def __init__(self, sections, areas=None, **kwargs): + super().__init__(sections, {} if areas is None else areas, **kwargs) @property def change_checkers(self): @@ -716,10 +719,48 @@ def test_changeschecker_constructor(): changelog = DummyChangelogChangesChecker("SECTIONS") assert changelog.sections == "SECTIONS" assert changelog.areas == {} + assert changelog._yamllint_config is None + assert changelog._yamllint_config_path is None with pytest.raises(NotImplementedError): changelog.change_checkers +def test_changeschecker_yamllint_config_passed(tmp_path): + config = YamlLintConfig("extends: default") + config_path = tmp_path / ".yamllint" + config_path.write_text("invalid config") + changelog = DummyChangelogChangesChecker( + "SECTIONS", + yamllint_config=config, + yamllint_config_path=config_path) + + assert changelog._yamllint_config is config + assert changelog._yamllint_config_path == config_path + assert changelog.yamllint_config is config + assert changelog.__dict__["yamllint_config"] is config + + +@pytest.mark.parametrize("exists", [True, False]) +@pytest.mark.parametrize("explicit_path", [True, False]) +def test_changeschecker_yamllint_config( + tmp_path, monkeypatch, exists, explicit_path): + monkeypatch.chdir(tmp_path) + config_path = tmp_path / ".yamllint" + if exists: + config_path.write_text( + "extends: default\nrules:\n line-length: {max: 140}\n") + changelog = DummyChangelogChangesChecker( + "SECTIONS", + yamllint_config_path=config_path if explicit_path else None) + + config = changelog.yamllint_config + + assert isinstance(config, YamlLintConfig) + assert config.rules["line-length"]["max"] == (140 if exists else 80) + assert changelog.__dict__["yamllint_config"] is config + assert changelog.yamllint_config is config + + def test_changeschecker_max_version_for_changes_section(patches): changelog = DummyChangelogChangesChecker("SECTIONS") patched = patches( @@ -1031,31 +1072,87 @@ def test_changeschecker_check_entry_content(content, expected): assert substring in result +@pytest.mark.parametrize( + "content,expected", + [(f"{'a' * 134} b", ()), + ("a" * 137, ()), + (f"{'a' * 135} b", + ("entry.rst:1: line too long (141 > 140 characters)", )), + (f"valid\n{'a' * 135} b\n{'a' * 136} c", + ("entry.rst:2: line too long (141 > 140 characters)", + "entry.rst:3: line too long (142 > 140 characters)")), + (":ref:`envoy_v3_api_field_extensions.bootstrap.reverse_tunnel." + "downstream_socket_interface.v3.DownstreamReverseConnection" + "SocketInterface.maintain_interval`", ()), + (f"key: {'a' * 137}", ()), + ("The downstream reverse-tunnel initiator\n" + "(``envoy.bootstrap.reverse_tunnel.downstream_socket_interface``) " + "now accepts\n" + ":ref:`maintain_interval " + "`\n" + "to control how often each host is re-checked and missing tunnels " + "are dialed.\n", + ("entry.rst:3: line too long (177 > 140 characters)", )), + ("valid \n", ()), + ("", ())]) +def test_changeschecker_check_entry_line_lengths( + tmp_path, content, expected): + changelog = DummyChangelogChangesChecker( + "SECTIONS", + yamllint_config=YamlLintConfig( + "extends: default\n" + "rules:\n" + " line-length:\n" + " max: 140\n" + " allow-non-breakable-words: true\n" + " allow-non-breakable-inline-mappings: true\n")) + path = tmp_path / "entry.rst" + path.write_text(content) + + assert ( + changelog.check_entry_line_lengths(path) + == tuple(f"{tmp_path}/{error}" for error in expected)) + + def test_changeschecker_check_entry_files(patches): changelog = DummyChangelogChangesChecker("SECTIONS") patched = patches( "AChangelogChangesChecker.check_entry_content", "AChangelogChangesChecker.check_entry_filename", + "AChangelogChangesChecker.check_entry_line_lengths", prefix="envoy.code.check.abstract.changelog") paths = [MagicMock(), MagicMock(), MagicMock()] - # path 0: filename error + content error + # path 0: filename error + content error + line length error # path 1: no errors - # path 2: content error only + # path 2: content error + line length errors filename_returns = ["FILENAME_ERR", None, None] content_returns = ["CONTENT_ERR", None, "CONTENT_ERR2"] + line_length_returns = [ + ("LINE_LENGTH_ERR", ), + (), + ("LINE_LENGTH_ERR2", "LINE_LENGTH_ERR3")] - with patched as (m_content, m_filename): + with patched as (m_content, m_filename, m_line_lengths): m_filename.side_effect = filename_returns m_content.side_effect = content_returns + m_line_lengths.side_effect = line_length_returns result = changelog.check_entry_files(paths) - assert result == ("FILENAME_ERR", "CONTENT_ERR", "CONTENT_ERR2") + assert ( + result + == ("FILENAME_ERR", "CONTENT_ERR", "LINE_LENGTH_ERR", + "CONTENT_ERR2", "LINE_LENGTH_ERR2", "LINE_LENGTH_ERR3")) assert ( m_filename.call_args_list == [[(p, ), {}] for p in paths]) assert ( m_content.call_args_list == [[(p, ), {}] for p in paths]) + assert ( + m_line_lengths.call_args_list + == [[(p, ), {}] for p in paths]) @pytest.mark.parametrize("is_current", [True, False]) @@ -1066,11 +1163,9 @@ def test_changelogstatus_entry_dir(patches, is_current): dict(new_callable=PropertyMock)), ("AChangelogStatus.project", dict(new_callable=PropertyMock)), - ("AChangelogStatus.version", - dict(new_callable=PropertyMock)), prefix="envoy.code.check.abstract.changelog") - with patched as (m_current, m_project, m_version): + with patched as (m_current, m_project): m_current.return_value = is_current m_project.return_value.changelogs.current_dir_path = ( MagicMock(name="CURRENT_DIR_PATH")) @@ -1079,7 +1174,6 @@ def test_changelogstatus_entry_dir(patches, is_current): if not is_current: assert result is None assert not m_project.called - assert not m_version.called return assert result == m_project.return_value.changelogs.current_dir_path assert not m_project.return_value.changelogs.changelog_path.called @@ -1088,7 +1182,6 @@ def test_changelogstatus_entry_dir(patches, is_current): .changelog_path.return_value .with_suffix.called) assert "entry_dir" not in status.__dict__ - assert not m_version.called @pytest.mark.parametrize("entry_dir_exists", [None, False, True])