Repository: relekang/python-semantic-release
Pull request #349 (2021-06-10 21:40:04)

Refactor `VersionDeclaration` to use `pathlib`
This extracts some refactoring out of #346 which I think are beneficials:

- Refactor `VersionDeclaration` to use `pathlib`
- Add a test to reproduce the out-of-order issue with `tomlkit` 0.7.2 + dotty-dict




Patch:
diff --git a/semantic_release/history/__init__.py b/semantic_release/history/__init__.py
index 96433c2..a069fc0 100644
--- a/semantic_release/history/__init__.py
+++ b/semantic_release/history/__init__.py
@@ -4,7 +4,8 @@ import csv
 import logging
 import re
 from abc import ABC, abstractmethod
-from typing import List, Optional, Set
+from pathlib import Path
+from typing import List, Optional, Set, Union
 
 import semver
 import tomlkit
@@ -24,8 +25,8 @@ logger = logging.getLogger(__name__)
 
 
 class VersionDeclaration(ABC):
-    def __init__(self, path: str):
-        self.path = path
+    def __init__(self, path: Union[str, Path]):
+        self.path = Path(path)
 
     @staticmethod
     def from_toml(config_str: str):
@@ -87,23 +88,21 @@ class TomlVersionDeclaration(VersionDeclaration):
         super().__init__(path)
         self.key = key
 
-    def _read(self):
-        with open(self.path, "r") as f:
-            return Dotty(tomlkit.loads(f.read()))
+    def _read(self) -> Dotty:
+        toml_doc = tomlkit.loads(self.path.read_text())
+        return Dotty(toml_doc)
 
     def parse(self) -> Set[str]:
-        config = self._read()
-        if self.key in config:
-            return {config.get(self.key)}
+        _config = self._read()
+        if self.key in _config:
+            return {_config.get(self.key)}
         return set()
 
-    def replace(self, new_version: str):
-        config = self._read()
-        version = self.key in config
-        if version:
-            config[self.key] = new_version
-            with open(self.path, "w") as f:
-                f.write(tomlkit.dumps(config))
+    def replace(self, new_version: str) -> None:
+        _config = self._read()
+        if self.key in _config:
+            _config[self.key] = new_version
+            self.path.write_text(tomlkit.dumps(_config))
 
 
 class PatternVersionDeclaration(VersionDeclaration):
@@ -134,8 +133,7 @@ class PatternVersionDeclaration(VersionDeclaration):
         should be the same version in each place), but it falls on the caller
         to check for this condition.
         """
-        with open(self.path, "r") as f:
-            content = f.read()
+        content = self.path.read_text()
 
         versions = {
             m.group(1) for m in re.finditer(self.pattern, content, re.MULTILINE)
@@ -156,8 +154,7 @@ class PatternVersionDeclaration(VersionDeclaration):
         :param new_version: The new version number as a string
         """
         n = 0
-        with open(self.path, "r") as f:
-            old_content = f.read()
+        old_content = self.path.read_text()
 
         def swap_version(m):
             nonlocal n
@@ -175,8 +172,7 @@ class PatternVersionDeclaration(VersionDeclaration):
             f"Writing new version number: path={self.path!r} pattern={self.pattern!r} num_matches={n!r}"
         )
 
-        with open(self.path, mode="w") as f:
-            f.write(new_content)
+        self.path.write_text(new_content)
 
 
 @LoggedFunction(logger)


Test patch:
diff --git a/tests/history/test_version.py b/tests/history/test_version.py
index ff40341..a8c111c 100644
--- a/tests/history/test_version.py
+++ b/tests/history/test_version.py
@@ -1,3 +1,4 @@
+from pathlib import Path
 from textwrap import dedent
 
 import mock
@@ -107,7 +108,7 @@ class TestVersionPattern:
         [
             (
                 "path:__version__",
-                "path",
+                Path("path"),
                 r'__version__ *[:=] *["\'](\d+\.\d+(?:\.\d+)?)["\']',
             ),
         ],
@@ -120,8 +121,8 @@ class TestVersionPattern:
     @pytest.mark.parametrize(
         "str, path, pattern",
         [
-            ("path:pattern", "path", r"pattern"),
-            ("path:Version: {version}", "path", r"Version: (\d+\.\d+(?:\.\d+)?)"),
+            ("path:pattern", Path("path"), r"pattern"),
+            ("path:Version: {version}", Path("path"), r"Version: (\d+\.\d+(?:\.\d+)?)"),
         ],
     )
     def test_from_pattern(self, str, path, pattern):
@@ -132,8 +133,8 @@ class TestVersionPattern:
     @pytest.mark.parametrize(
         "str, path, key",
         [
-            ("path:some.toml.key", "path", r"some.toml.key"),
-            ("path:some:other:toml.key", "path", r"some:other:toml.key"),
+            ("path:some.toml.key", Path("path"), r"some.toml.key"),
+            ("path:some:other:toml.key", Path("path"), r"some:other:toml.key"),
         ],
     )
     def test_from_toml(self, str, path, key):
@@ -207,15 +208,15 @@ class TestVersionPattern:
     @pytest.mark.parametrize(
         "key, content, hits",
         [
-            (r"root", 'root = "test"', {"test"}),
-            (r"tool.poetry.version", '[tool.poetry]\nversion = "0.1.0"', {"0.1.0"}),
+            ("root", 'root = "test"', {"test"}),
+            ("tool.poetry.version", '[tool.poetry]\nversion = "0.1.0"', {"0.1.0"}),
         ],
     )
     def test_toml_parse(self, tmp_path, key, content, hits):
         path = tmp_path / "pyproject.toml"
         path.write_text(content)
 
-        declaration = TomlVersionDeclaration(str(path), key)
+        declaration = TomlVersionDeclaration(path, key)
         assert declaration.parse() == hits
 
     @pytest.mark.parametrize(
@@ -224,17 +225,58 @@ class TestVersionPattern:
             (r"root", "", ""),
             (r"root", 'root = "test"', 'root = "-"'),
             (
-                r"tool.poetry.version",
-                "[tool.poetry]\n"
-                'version = "0.1.0"\n'
-                "[tool.poetry.dependencies.pylint]\n"
-                'version = "^2.5.3"\n'
-                "optional = true\n",
-                "[tool.poetry]\n"
-                'version = "-"\n'
-                "[tool.poetry.dependencies.pylint]\n"
-                'version = "^2.5.3"\n'
-                "optional = true\n",
+                "tool.poetry.version",
+                dedent(
+                    """
+                    [tool.poetry]
+                    version = "0.1.0"
+                    [tool.poetry.dependencies.pylint]
+                    version = "^2.5.3"
+                    optional = true
+                    """
+                ),
+                dedent(
+                    """
+                    [tool.poetry]
+                    version = "-"
+                    [tool.poetry.dependencies.pylint]
+                    version = "^2.5.3"
+                    optional = true
+                    """
+                ),
+            ),
+            (
+                "tool.poetry.version",
+                dedent(
+                    """
+                    [tool.poetry]
+                    name = "my-package"
+                    version = "0.1.0"
+                    description = "A super package"
+                    
+                    [build-system]
+                    requires = ["poetry-core>=1.0.0"]
+                    build-backend = "poetry.core.masonry.api"
+                    
+                    [tool.semantic_release]
+                    version_toml = "pyproject.toml:tool.poetry.version"
+                    """
+                ),
+                dedent(
+                    """
+                    [tool.poetry]
+                    name = "my-package"
+                    version = "-"
+                    description = "A super package"
+                    
+                    [build-system]
+                    requires = ["poetry-core>=1.0.0"]
+                    build-backend = "poetry.core.masonry.api"
+                    
+                    [tool.semantic_release]
+                    version_toml = "pyproject.toml:tool.poetry.version"
+                    """
+                ),
             ),
         ],
     )
@@ -267,7 +309,7 @@ class TestVersionPattern:
                         version_variable = "path:__version__"
                         """,
             patterns=[
-                ("path", r'__version__ *[:=] *["\'](\d+\.\d+(?:\.\d+)?)["\']'),
+                (Path("path"), r'__version__ *[:=] *["\'](\d+\.\d+(?:\.\d+)?)["\']'),
             ],
         ),
         dict(
@@ -276,8 +318,8 @@ class TestVersionPattern:
                         version_variable = "path1:var1,path2:var2"
                         """,
             patterns=[
-                ("path1", r'var1 *[:=] *["\'](\d+\.\d+(?:\.\d+)?)["\']'),
-                ("path2", r'var2 *[:=] *["\'](\d+\.\d+(?:\.\d+)?)["\']'),
+                (Path("path1"), r'var1 *[:=] *["\'](\d+\.\d+(?:\.\d+)?)["\']'),
+                (Path("path2"), r'var2 *[:=] *["\'](\d+\.\d+(?:\.\d+)?)["\']'),
             ],
         ),
         dict(
@@ -289,8 +331,8 @@ class TestVersionPattern:
                         ]
                         """,
             patterns=[
-                ("path1", r'var1 *[:=] *["\'](\d+\.\d+(?:\.\d+)?)["\']'),
-                ("path2", r'var2 *[:=] *["\'](\d+\.\d+(?:\.\d+)?)["\']'),
+                (Path("path1"), r'var1 *[:=] *["\'](\d+\.\d+(?:\.\d+)?)["\']'),
+                (Path("path2"), r'var2 *[:=] *["\'](\d+\.\d+(?:\.\d+)?)["\']'),
             ],
         ),
         dict(
@@ -299,7 +341,7 @@ class TestVersionPattern:
                         version_pattern = "path:pattern"
                         """,
             patterns=[
-                ("path", "pattern"),
+                (Path("path"), "pattern"),
             ],
         ),
         dict(
@@ -308,8 +350,8 @@ class TestVersionPattern:
                         version_pattern = "path1:pattern1,path2:pattern2"
                         """,
             patterns=[
-                ("path1", "pattern1"),
-                ("path2", "pattern2"),
+                (Path("path1"), "pattern1"),
+                (Path("path2"), "pattern2"),
             ],
         ),
         dict(
@@ -321,8 +363,8 @@ class TestVersionPattern:
                         ]
                         """,
             patterns=[
-                ("path1", "pattern1"),
-                ("path2", "pattern2"),
+                (Path("path1"), "pattern1"),
+                (Path("path2"), "pattern2"),
             ],
         ),
     ],
diff --git a/tests/test_cli.py b/tests/test_cli.py
index aec9fc4..915c9fa 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -825,7 +825,9 @@ def test_changelog_should_call_functions(mocker, runner):
 
 
 def test_overload_by_cli(mocker, runner):
-    mock_open = mocker.patch("semantic_release.history.open", mock_version_file)
+    mock_read_text = mocker.patch(
+        "semantic_release.history.Path.read_text", mock_version_file
+    )
     runner.invoke(
         main,
         [
@@ -837,8 +839,8 @@ def test_overload_by_cli(mocker, runner):
         ],
     )
 
-    mock_open.assert_called_once_with("my_version_path", "r")
-    mock_open.reset_mock()
+    mock_read_text.assert_called_once_with()
+    mock_read_text.reset_mock()
 
 
 def test_changelog_noop(mocker):