Preserve per-entry changelog errors on parse failure and switch envoy.code.check to shared entry constants

py/envoy.base.utils/envoy/base/utils/abstract/project/changelog.py

@@ -27,7 +27,9 @@
 CHANGELOG_PATH_GLOB = "changelogs/*.*.*.yaml"
 CHANGELOG_PATH_FMT = "changelogs/{version}.yaml"
 CHANGELOG_CURRENT_PATH = "changelogs/current.yaml"
+CHANGELOG_ENTRY_GLOB = "*/*.rst"
 CHANGELOG_SECTIONS_PATH = "changelogs/sections.yaml"
+ENTRY_SEPARATOR = "__"
 CHANGELOG_SUMMARY_PATH = "changelogs/summary.md"
 CHANGELOG_URL_TPL = (
     "https://raw.githubusercontent.com/envoyproxy/envoy/"

py/envoy.code.check/envoy/code/check/BUILD

@@ -5,7 +5,7 @@ toolshed_library(
         "//py/deps:reqs#abstracts",
         "//py/deps:reqs#aio-core",
         "//py/deps:reqs#aio-run-checker",
-        "//py/deps:reqs#envoy-base-utils",
+        "//py/envoy.base.utils/envoy/base/utils",
         "//py/deps:reqs#flake8",
         "//py/deps:reqs#packaging",
         "//py/deps:reqs#yamllint",

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

@@ -12,15 +12,15 @@
 from aio.core.functional import async_property
 
 from envoy.base import utils
+from envoy.base.utils.abstract.project.changelog import (
+    CHANGELOG_ENTRY_GLOB,
+    ENTRY_SEPARATOR,
+)
 from envoy.code.check import abstract, interface
 
 
 MAX_VERSION_FOR_CHANGES_SECTION = "1.16"
 
-# TODO: Replace with imports from envoy.base.utils once PR 2 lands
-ENTRY_SEPARATOR = "__"
-CHANGELOG_ENTRY_GLOB = "*/*.rst"
-
 
 @abstracts.implementer(interface.IChangelogChangesChecker)
 class AChangelogChangesChecker(metaclass=abstracts.Abstraction):
@@ -196,14 +196,15 @@ def entry_dir(self) -> pathlib.Path | None:
 
     @async_property(cache=True)
     async def errors(self) -> tuple[str, ...]:
+        entry_errors = await self.check_entry_files()
         try:
             return (
                 *self.check_version(),
                 *await self.check_date(),
                 *await self.check_sections(),
-                *await self.check_entry_files())
+                *entry_errors)
         except utils.exceptions.ChangelogParseError as e:
-            return (f"{self.version}: {e}", )
+            return (*entry_errors, f"{self.version}: {e}")
 
     @async_property
     async def invalid_date(self) -> str | None:

py/envoy.code.check/tests/BUILD

@@ -5,7 +5,7 @@ toolshed_tests(
         "//py/deps:reqs#abstracts",
         "//py/deps:reqs#aio-core",
         "//py/deps:reqs#aio-run-checker",
-        "//py/deps:reqs#envoy-base-utils",
+        "//py/envoy.base.utils/envoy/base/utils",
         "//py/deps:reqs#flake8",
         "//py/deps:reqs#packaging",
         "//py/deps:reqs#pep8-naming",

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

@@ -1,5 +1,4 @@
 
-import pathlib
 import types
 from unittest.mock import AsyncMock, MagicMock, PropertyMock
 
@@ -279,30 +278,29 @@ async def test_changelogstatus_errors(iters, patches, raises):
             with pytest.raises(raises):
                 await status.errors
             return
+        expected_errors = (
+            (*version_errors, *date_errors, *sections_errors,
+             *entry_files_errors)
+            if not raises
+            else (
+                *entry_files_errors,
+                f"{m_version.return_value}: {error}"))
         assert (
             await status.errors
-            == ((*version_errors, *date_errors, *sections_errors,
-                 *entry_files_errors)
-                if not raises
-                else (f"{m_version.return_value}: {error}", ))
+            == expected_errors
             == getattr(
                 status,
                 check.AChangelogStatus.errors.cache_name)["errors"])
 
-    for provider in [m_versions, m_date]:
-        assert (
-            provider.call_args
-            == [(), {}])
+    providers = [m_versions, m_date, m_entry_files]
     if not raises:
+        providers.append(m_sections)
+    for provider in providers:
         assert (
-            m_sections.call_args
-            == [(), {}])
-        assert (
-            m_entry_files.call_args
+            provider.call_args
             == [(), {}])
-    else:
+    if raises:
         assert not m_sections.called
-        assert not m_entry_files.called
 
 
 @pytest.mark.parametrize("raises", [None, Exception, ValueError])
@@ -771,13 +769,19 @@ def test_changeschecker_check_section_name(
 
 def test_changeschecker_check_entry_filename_valid():
     changelog = DummyChangelogChangesChecker({"bug_fixes": MagicMock()})
-    path = pathlib.Path("changelogs/current/bug_fixes/myarea__myslug.rst")
+    path = MagicMock()
+    path.parent.name = "bug_fixes"
+    path.stem = "myarea__myslug"
+    path.suffix = ".rst"
     assert changelog.check_entry_filename(path) is None
 
 
 def test_changeschecker_check_entry_filename_invalid_section():
     changelog = DummyChangelogChangesChecker({"bug_fixes": MagicMock()})
-    path = pathlib.Path("changelogs/current/weird_section/area__slug.rst")
+    path = MagicMock()
+    path.parent.name = "weird_section"
+    path.stem = "area__slug"
+    path.suffix = ".rst"
     result = changelog.check_entry_filename(path)
     assert result is not None
     assert "weird_section" in result
@@ -786,7 +790,10 @@ def test_changeschecker_check_entry_filename_invalid_section():
 
 def test_changeschecker_check_entry_filename_wrong_extension():
     changelog = DummyChangelogChangesChecker({"bug_fixes": MagicMock()})
-    path = pathlib.Path("changelogs/current/bug_fixes/area__slug.txt")
+    path = MagicMock()
+    path.parent.name = "bug_fixes"
+    path.stem = "area__slug"
+    path.suffix = ".txt"
     result = changelog.check_entry_filename(path)
     assert result is not None
     assert ".txt" in result
@@ -795,23 +802,32 @@ def test_changeschecker_check_entry_filename_wrong_extension():
 
 def test_changeschecker_check_entry_filename_no_separator():
     changelog = DummyChangelogChangesChecker({"bug_fixes": MagicMock()})
-    path = pathlib.Path("changelogs/current/bug_fixes/areaslug.rst")
+    path = MagicMock()
+    path.parent.name = "bug_fixes"
+    path.stem = "areaslug"
+    path.suffix = ".rst"
     result = changelog.check_entry_filename(path)
     assert result is not None
     assert "__" in result
 
 
 def test_changeschecker_check_entry_filename_multiple_separators():
     changelog = DummyChangelogChangesChecker({"bug_fixes": MagicMock()})
-    path = pathlib.Path("changelogs/current/bug_fixes/area__slug__extra.rst")
+    path = MagicMock()
+    path.parent.name = "bug_fixes"
+    path.stem = "area__slug__extra"
+    path.suffix = ".rst"
     result = changelog.check_entry_filename(path)
     assert result is not None
     assert "__" in result
 
 
 def test_changeschecker_check_entry_filename_empty_area():
     changelog = DummyChangelogChangesChecker({"bug_fixes": MagicMock()})
-    path = pathlib.Path("changelogs/current/bug_fixes/__slug.rst")
+    path = MagicMock()
+    path.parent.name = "bug_fixes"
+    path.stem = "__slug"
+    path.suffix = ".rst"
     result = changelog.check_entry_filename(path)
     assert result is not None
     assert "Area" in result
@@ -820,7 +836,10 @@ def test_changeschecker_check_entry_filename_empty_area():
 
 def test_changeschecker_check_entry_filename_empty_slug():
     changelog = DummyChangelogChangesChecker({"bug_fixes": MagicMock()})
-    path = pathlib.Path("changelogs/current/bug_fixes/area__.rst")
+    path = MagicMock()
+    path.parent.name = "bug_fixes"
+    path.stem = "area__"
+    path.suffix = ".rst"
     result = changelog.check_entry_filename(path)
     assert result is not None
     assert "Slug" in result