changelog: validate rst entry line lengths

py/envoy.code.check/envoy/code/check/abstract/changelog.py

@@ -22,6 +22,8 @@
 
 
 MAX_VERSION_FOR_CHANGES_SECTION = "1.16"
+# YAML block scalars add four spaces to Envoy's 140-character limit.
+MAX_CHANGELOG_ENTRY_LINE_LENGTH = 136
 VALID_CHANGELOG_AREA_RE = re.compile(r"^[a-z0-9_\-/]+$")
 VALID_CHANGELOG_AREA_PATTERN = r"[a-z0-9_\-/]+"
 
@@ -174,6 +176,18 @@ 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, ...]:
+        return tuple(
+            f"{path}:{line_number}: Changelog entry line is "
+            f"{len(line)} characters (maximum "
+            f"{MAX_CHANGELOG_ENTRY_LINE_LENGTH})"
+            for line_number, line
+            in enumerate(path.read_text().splitlines(), start=1)
+            if (len(line) > MAX_CHANGELOG_ENTRY_LINE_LENGTH
+                and len(line.split()) > 1))
+
     def check_entry_files(
             self,
             paths: list[pathlib.Path]) -> tuple[str, ...]:
@@ -183,6 +197,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)
 
 
@@ -231,10 +246,7 @@ def duplicate_current(self) -> bool:
     def entry_dir(self) -> pathlib.Path | None:
         if not self.is_current:
             return None
-        return (
-            self.project.changelogs
-                        .changelog_path(self.version)
-                        .with_suffix(""))
+        return self.project.changelogs.current_dir_path
 
     @async_property(cache=True)
     async def errors(self) -> tuple[str, ...]:
@@ -305,7 +317,7 @@ async def check_date(self) -> tuple[str, ...]:
         # In the entries layout the current changelog has no real date; it is
         # synthesized as `Pending` until `write_version` bakes a dated file,
         # so there is nothing to validate here.
-        if self.is_current and self.project.changelogs.entries_layout:
+        if self.is_current:
             return ()
         errors = []
         if invalid_date := await self.invalid_date:

py/envoy.code.check/tests/test_abstract_changelog.py

@@ -1030,31 +1030,66 @@ 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: Changelog entry line is 137 characters "
+       "(maximum 136)", )),
+     (f"valid\n{'a' * 135} b\n{'a' * 136} c",
+      ("entry.rst:2: Changelog entry line is 137 characters "
+       "(maximum 136)",
+       "entry.rst:3: Changelog entry line is 138 characters "
+       "(maximum 136)"))])
+def test_changeschecker_check_entry_line_lengths(
+        tmp_path, content, expected):
+    changelog = DummyChangelogChangesChecker("SECTIONS")
+    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])
@@ -1065,32 +1100,19 @@ 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
         result = status.entry_dir
 
     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
-                     .changelog_path.return_value
-                     .with_suffix.return_value))
-    assert (
-        m_project.return_value.changelogs.changelog_path.call_args
-        == [(m_version.return_value, ), {}])
-    assert (
-        (m_project.return_value.changelogs
-                  .changelog_path.return_value
-                  .with_suffix.call_args)
-        == [("", ), {}])
+        == m_project.return_value.changelogs.current_dir_path)
     assert "entry_dir" not in status.__dict__