envoy.github.release: three correctness fixes (version regex, status-before-stream, exception chain)

py/envoy.github.release/envoy/github/release/assets.py

@@ -59,15 +59,16 @@ async def save(
             download: aiohttp.ClientResponse) -> dict[
                 str, str | pathlib.Path]:
         outfile = self.path.joinpath(asset_type, name)
-        outfile.parent.mkdir(exist_ok=True)
-        async with stream.writer(outfile) as f:
-            await f.stream_bytes(download)
         result: dict[str, str | pathlib.Path] = dict(
             name=name,
             outfile=outfile)
         if download.status != 200:
             result["error"] = self.fail(
                 f"Failed downloading, got response:\n{download}")
+            return result
+        outfile.parent.mkdir(exist_ok=True)
+        async with stream.writer(outfile) as f:
+            await f.stream_bytes(download)
         return result
 
 

py/envoy.github.release/envoy/github/release/manager.py

@@ -22,7 +22,7 @@
 @abstracts.implementer(AGithubReleaseManager)
 class GithubReleaseManager:
 
-    _version_re = r"v(\w+)"
+    _version_re = r"^v(\d+\.\d+\.\d+.*)$"
     _version_format = "v{version}"
 
     async def __aenter__(self) -> AGithubReleaseManager:
@@ -101,10 +101,10 @@ def format_version(
     def parse_version(
             self,
             version: str) -> packaging.version.Version | None:
-        _version = self.version_re.sub(r"\1", version)
-        if _version:
+        m = self.version_re.fullmatch(version)
+        if m:
             try:
-                return packaging.version.Version(_version)
+                return packaging.version.Version(m.group(1))
             except packaging.version.InvalidVersion:
                 pass
         self.log.warning(f"Unable to parse version: {version}")

py/envoy.github.release/envoy/github/release/release.py

@@ -198,7 +198,7 @@ async def push(
                     response["assets"].append(result)
                     self.log.info(f"Release file uploaded {result['name']}")
         except ConcurrentError as e:
-            raise e.args[0]
+            raise e.args[0] from e
         if not response["errors"]:
             self.log.success(f"Assets uploaded: {self.version}")
         return response

py/envoy.github.release/tests/test_assets.py

@@ -205,24 +205,27 @@ async def test_fetcher_save(patches, status):
             m_fail.call_args
             == [(f"Failed downloading, got response:\n{download}", ), {}])
         expected["error"] = m_fail.return_value
+        # stream.writer must NOT be called on error
+        assert not m_stream.writer.called
+        assert not outfile.parent.mkdir.called
     else:
         assert not m_fail.called
+        assert (
+            outfile.parent.mkdir.call_args
+            == [(), dict(exist_ok=True)])
+        writer = m_stream.writer
+        assert (
+            writer.call_args
+            == [(outfile, ), {}])
+        stream_bytes = writer.return_value.__aenter__.return_value.stream_bytes
+        assert (
+            stream_bytes.call_args
+            == [(download, ), {}])
 
     assert result == expected
     assert (
         m_path.return_value.joinpath.call_args
         == [('ASSET TYPE', 'NAME'), {}])
-    assert (
-        outfile.parent.mkdir.call_args
-        == [(), dict(exist_ok=True)])
-    writer = m_stream.writer
-    assert (
-        writer.call_args
-        == [(outfile, ), {}])
-    stream_bytes = writer.return_value.__aenter__.return_value.stream_bytes
-    assert (
-        stream_bytes.call_args
-        == [(download, ), {}])
 
 
 def test_pusher_constructor(patches):

py/envoy.github.release/tests/test_manager.py

@@ -52,7 +52,7 @@ def test_release_manager_constructor(
     assert releaser._github == github
     assert releaser._session == session
 
-    assert releaser._version_re == r"v(\w+)"
+    assert releaser._version_re == r"^v(\d+\.\d+\.\d+.*)$"
 
 
 async def test_release_manager_async_contextmanager(patches):
@@ -335,11 +335,11 @@ def test_release_manager_format_version():
         == [(), dict(version="VERSION")])
 
 
-@pytest.mark.parametrize("version", [None, 0, "", "1.2.3"])
+@pytest.mark.parametrize("matched", [None, "1.2.3"])
 @pytest.mark.parametrize(
     "raises",
     [None, BaseException, packaging.version.InvalidVersion])
-def test_release_manager_parse_version(patches, version, raises):
+def test_release_manager_parse_version(patches, matched, raises):
     releaser = GithubReleaseManager("PATH", "REPOSITORY")
     patched = patches(
         "packaging.version.Version",
@@ -348,33 +348,94 @@ def test_release_manager_parse_version(patches, version, raises):
         prefix="envoy.github.release.manager")
 
     with patched as (m_packaging, m_log, m_version):
-        m_version.return_value.sub.return_value = version
+        if matched:
+            m_match = MagicMock()
+            m_match.group.return_value = matched
+            m_version.return_value.fullmatch.return_value = m_match
+        else:
+            m_version.return_value.fullmatch.return_value = None
         if raises:
             m_packaging.side_effect = raises()
 
-        if version and raises == BaseException:
+        if matched and raises == BaseException:
             with pytest.raises(BaseException):
                 releaser.parse_version("VERSION")
         else:
             assert (
                 releaser.parse_version("VERSION")
                 == (None
-                    if not version or raises
+                    if not matched or raises
                     else m_packaging.return_value))
 
     assert (
-        m_version.return_value.sub.call_args
-        == [(r"\1", "VERSION"), {}])
-    if version:
+        m_version.return_value.fullmatch.call_args
+        == [("VERSION", ), {}])
+    if matched:
+        assert (
+            m_match.group.call_args
+            == [(1, ), {}])
         assert (
             m_packaging.call_args
-            == [(m_version.return_value.sub.return_value, ), {}])
+            == [(m_match.group.return_value, ), {}])
     else:
         assert not m_packaging.called
 
-    if not version or raises and raises != BaseException:
+    if not matched or raises and raises != BaseException:
         assert (
             m_log.return_value.warning.call_args
             == [("Unable to parse version: VERSION", ), {}])
     else:
         assert not m_log.called
+
+
+@pytest.mark.parametrize(
+    "version,expected",
+    [("v1.19.0", packaging.version.Version("1.19.0")),
+     ("v0.0.1", packaging.version.Version("0.0.1")),
+     ("v1.2.3-rc1", packaging.version.Version("1.2.3rc1")),
+     ("v1.2.3.dev0", packaging.version.Version("1.2.3.dev0")),
+     ("v1", None),
+     ("notaversion", None)])
+def test_release_manager_parse_version_regex(patches, version, expected):
+    """Integration test: parse_version with the real version_re regex."""
+    releaser = GithubReleaseManager("PATH", "REPOSITORY")
+    patched = patches(
+        ("GithubReleaseManager.log", dict(new_callable=PropertyMock)),
+        prefix="envoy.github.release.manager")
+
+    with patched as (m_log, ):
+        result = releaser.parse_version(version)
+
+    assert result == expected
+    if expected is None:
+        assert (
+            m_log.return_value.warning.call_args
+            == [(f"Unable to parse version: {version}", ), {}])
+    else:
+        assert not m_log.called
+
+
+async def test_release_manager_latest_minors(patches):
+    """Integration test: latest dict uses correct per-minor bucketing."""
+    releaser = GithubReleaseManager("PATH", "REPOSITORY")
+    patched = patches(
+        ("GithubReleaseManager.releases", dict(new_callable=PropertyMock)),
+        prefix="envoy.github.release.manager")
+
+    _versions = [
+        dict(tag_name=v)
+        for v in ("v1.18.0", "v1.19.0", "v1.19.1", "v2.0.0")]
+
+    with patched as (m_releases, ):
+        m_releases.side_effect = AsyncMock(return_value=_versions)
+        result = await releaser.latest
+
+    assert result == {
+        "1.18.0": packaging.version.Version("1.18.0"),
+        "1.18": packaging.version.Version("1.18.0"),
+        "1.19.0": packaging.version.Version("1.19.0"),
+        "1.19.1": packaging.version.Version("1.19.1"),
+        "1.19": packaging.version.Version("1.19.1"),
+        "2.0.0": packaging.version.Version("2.0.0"),
+        "2.0": packaging.version.Version("2.0.0"),
+    }

py/envoy.github.release/tests/test_release.py

@@ -514,7 +514,7 @@ async def _pusher(path):
                 BaseException
                 if raises == BaseException
                 else SomeError)
-            with pytest.raises(exception):
+            with pytest.raises(exception) as exc_info:
                 await release.push(artefacts)
         else:
             assert await release.push(artefacts) == expected
@@ -528,6 +528,9 @@ async def _pusher(path):
             m_pusher.return_value.call_args_list
             == [[(release, 'ARTEFACTS0'), {}]])
         assert not m_log.return_value.info.called
+        if raises == tasks.ConcurrentError:
+            assert isinstance(
+                exc_info.value.__cause__, tasks.ConcurrentError)
     else:
         assert (
             m_pusher.return_value.call_args_list