Refactor the ArtifactInfo class (teemtee#4526)

An effort towards resolving: teemtee#4055 (part-1)

following changes made:
1. All subclasses of `ArtifactInfo` now removed
2. Updated attributs of `ArtifactInfo` to what was discussed during the
artifact plugin design sync (`id`, `version`, `name`, `location`,
`provider`). The `__str__` of this container represents:
`<provider-id>-<nvra>`
3. Introduced `Version` object and its subclass (`RpmVersion` with its
factory methods) to represent `nvra` & `nevra` construction
4. Refactored all classes (src and tests) where the above was referenced
5. Removed `ArtifactInfoT` generic since see point 1.

Author: AthreyVinay

tests/unit/artifact/__init__.py

@@ -42,7 +42,7 @@ def mock_api(method, *args, **kwargs):
         if method == "listBuildRPMs":
             return mock_rpms
         if method == "getBuild":
-            return {"id": MOCK_BUILD_ID_KOJI_BREW}
+            return {"id": MOCK_BUILD_ID_KOJI_BREW, "package_name": "test-package"}
         return None
 
     mock_call_api.side_effect = mock_api
@@ -86,11 +86,13 @@ def mock_api(method, *args, **kwargs):
             return [{"build_id": MOCK_BUILD_ID_KOJI_BREW}] if has_build else []
         if method == "listBuildRPMs":
             return mock_rpms
+        if method == "getBuild":
+            return {"id": MOCK_BUILD_ID_KOJI_BREW, "package_name": "test-package"}
         if method == "getTaskDescendents":
             task_id = args[0] if args else kwargs.get('taskID')
             return {str(task_id): None, str(task_id + 1): None}
         if method == "listTaskOutput":
-            return ["foo.rpm", "bar.rpm"]
+            return ["foo-1.0-1.fc43.x86_64.rpm", "bar-2.0-1.fc43.x86_64.rpm"]
         return None
 
     mock_call_api.side_effect = mock_api

tests/unit/artifact/conftest.py

@@ -2,14 +2,19 @@
 
 import pytest
 
+import tmt.plugins
 from tmt.steps.prepare import install
 from tmt.steps.prepare.artifact import get_artifact_provider
 from tmt.steps.prepare.artifact.providers import koji as koji_module
 from tmt.steps.prepare.artifact.providers.brew import BrewArtifactProvider
-from tmt.steps.prepare.artifact.providers.copr_repository import CoprRepositoryProvider
 from tmt.steps.prepare.artifact.providers.koji import KojiArtifactProvider
 
 
+@pytest.fixture(scope='session')
+def _load_plugins():
+    tmt.plugins.explore(tmt.Logger.create())
+
+
 @pytest.fixture
 def mock_pathinfo():
     mock_pathinfo = MagicMock()
@@ -23,33 +28,31 @@ def mock_koji(mock_pathinfo):
     mock_koji = MagicMock()
     mock_koji.PathInfo.return_value = mock_pathinfo
 
+    def mock_initialize_session(self, api_url=None, top_url=None):
+        self._top_url = top_url or "http://koji.example.com/"
+        self._api_url = api_url or "http://koji.example.com/kojihub"
+        return MagicMock()
+
     with (
-        patch.object(KojiArtifactProvider, "_initialize_session", return_value=MagicMock()),
-        patch.object(
-            KojiArtifactProvider,
-            "_rpm_url",
-            side_effect=lambda rpm: f"http://koji.example.com/{rpm['name']}.rpm",
-        ),
+        patch.object(KojiArtifactProvider, "_initialize_session", mock_initialize_session),
         patch.object(koji_module, "koji", mock_koji),
     ):
         yield mock_koji
 
 
 @pytest.fixture
 def mock_brew(mock_koji):
-    with (
-        patch.object(BrewArtifactProvider, "_initialize_session", return_value=MagicMock()),
-        patch.object(
-            BrewArtifactProvider,
-            "_rpm_url",
-            side_effect=lambda rpm: f"http://brew.example.com/{rpm['name']}.rpm",
-        ),
-    ):
+    def mock_initialize_session(self, api_url=None, top_url=None):
+        self._top_url = top_url or "http://brew.example.com/"
+        self._api_url = api_url or "http://brew.example.com/brewhub"
+        return MagicMock()
+
+    with patch.object(BrewArtifactProvider, "_initialize_session", mock_initialize_session):
         yield mock_koji
 
 
 @pytest.fixture
-def artifact_provider(root_logger):
+def artifact_provider(root_logger, _load_plugins):
     def get_provider(provider_id: str, repository_priority: int = 50):
         provider_class = get_artifact_provider(provider_id)
         return provider_class(

tests/unit/artifact/test_base.py

@@ -1,26 +1,22 @@
 import re
 from unittest.mock import MagicMock
 
-from tmt.steps.prepare.artifact.providers import ArtifactInfo, ArtifactProvider
+from tmt.steps.prepare.artifact.providers import ArtifactInfo, ArtifactProvider, Version
 
 
-class MockArtifactInfo(ArtifactInfo):
-    @property
-    def id(self) -> str:
-        return "mock.rpm"
-
-    @property
-    def location(self) -> str:
-        return "http://example.com/mock.rpm"
-
-
-class MockProvider(ArtifactProvider[MockArtifactInfo]):
+class MockProvider(ArtifactProvider):
     def _extract_provider_id(self, raw_provider_id: str) -> str:
         return raw_provider_id.split(":", 1)[1]
 
     @property
     def artifacts(self):
-        return [MockArtifactInfo(_raw_artifact={})]
+        return [
+            ArtifactInfo(
+                version=Version(name="mock", version="1.0", release="1", arch="x86_64"),
+                location="http://example.com/mock-1.0-1.x86_64.rpm",
+                provider=self,
+            )
+        ]
 
     def _download_artifact(self, artifact, guest, destination):
         destination.write_text("ok")
@@ -43,8 +39,7 @@ def test_download_artifacts(tmp_path, root_logger):
     provider = MockProvider("mock:123", repository_priority=50, logger=root_logger)
 
     paths = provider.fetch_contents(guest, tmp_path, [])
-
-    file_path = tmp_path / "mock.rpm"
+    file_path = tmp_path / "mock-1.0-1.x86_64.rpm"
     assert file_path in paths
     assert file_path.exists()
     assert file_path.read_text() == "ok"

tests/unit/artifact/test_brew.py

@@ -38,7 +38,9 @@ def test_brew_valid_draft_build(mock_brew, mock_call_api, artifact_provider):
         for i in range(2)
     ]
     mock_call_api.side_effect = (
-        lambda method, *a, **kw: mock_rpms if method == "listBuildRPMs" else {"id": draft_id}
+        lambda method, *a, **kw: mock_rpms
+        if method == "listBuildRPMs"
+        else {"id": draft_id, "package_name": "test-package"}
     )
 
     provider = artifact_provider(f"brew.build:{draft_id}")
@@ -53,7 +55,6 @@ def test_brew_valid_task_id_scratch_build(mock_brew, mock_call_api, artifact_pro
 
     provider = artifact_provider(f"brew.task:{task_id}")
     assert isinstance(provider, BrewTask)
-    provider._top_url = "http://brew.example.com"
     tasks = list(provider._get_task_children(task_id))
 
     assert len(tasks) == 2

tests/unit/artifact/test_copr_build.py

@@ -15,7 +15,7 @@ def test_copr_valid_build_non_pulp(artifact_provider):
     assert not provider.is_pulp
 
     for artifact in provider.artifacts:
-        assert artifact._raw_artifact["url"].startswith(provider.result_url)
+        assert artifact.location.startswith(provider.result_url)
 
 
 def test_copr_pulp_build(artifact_provider, monkeypatch):
@@ -29,8 +29,8 @@ def test_copr_pulp_build(artifact_provider, monkeypatch):
     assert len(artifacts) == len(MOCK_RPMS_PULP)
 
     for artifact in artifacts:
-        first_letter = artifact._raw_artifact["name"][0]
-        assert f"/Packages/{first_letter}/" in artifact._raw_artifact["url"]
+        first_letter = artifact.name[0]
+        assert f"/Packages/{first_letter}/" in artifact.location
 
 
 def test_copr_invalid_build_id(artifact_provider):

tests/unit/artifact/test_file.py

@@ -3,24 +3,20 @@
 import pytest
 
 from tmt.log import Logger
-from tmt.steps.prepare.artifact.providers.file import (
-    PackageAsFileArtifactInfo,
-    PackageAsFileArtifactProvider,
-)
+from tmt.steps.prepare.artifact.providers import RpmVersion
 from tmt.utils import Path
 
 
 @pytest.mark.parametrize(
     ("pattern", "expected_names"),
     [
-        ("foo.rpm", {"foo.rpm"}),
-        ("package-*.rpm", {f"package-{i}.rpm" for i in range(3)}),
+        ("foo-1.0-1.fc43.x86_64.rpm", {"foo-1.0-1.fc43.x86_64"}),
+        ("package-*.rpm", {f"package-{i}-1.0-1.fc43.x86_64" for i in range(3)}),
     ],
 )
 def test_file_artifact_provider_patterns(tmp_path, pattern, expected_names, artifact_provider):
-    # Create test files
     for name in expected_names:
-        (tmp_path / name).touch()
+        (tmp_path / f"{name}.rpm").touch()
 
     provider = artifact_provider(f"file:{tmp_path}/{pattern}")
     artifacts = provider.artifacts
@@ -35,26 +31,65 @@ def test_file_artifact_provider_deduplicates_globs(tmp_path, caplog, artifact_pr
     for dirname in ("foo", "bar"):
         subdir = tmp_path / dirname
         subdir.mkdir()
-        (subdir / "baz.rpm").touch()
+        (subdir / "baz-1.0-1.fc43.x86_64.rpm").touch()
 
-    provider = artifact_provider(f"file:{tmp_path}/*/baz.rpm")
+    provider = artifact_provider(f"file:{tmp_path}/*/baz-1.0-1.fc43.x86_64.rpm")
     artifacts = provider.artifacts
 
     assert len(artifacts) == 1
-    assert artifacts[0].id == "baz.rpm"
+    assert artifacts[0].id == "baz-1.0-1.fc43.x86_64"
     assert "Duplicate artifact" in caplog.text
 
 
 def test_download_artifact(tmp_path, artifact_provider):
     # Test file download
-    test_file = tmp_path / "foo.rpm"
+    test_file = tmp_path / "foo-1.0-1.fc43.x86_64.rpm"
     test_file.touch()
     file_provider = artifact_provider(f"file:{test_file}")
     guest = MagicMock()
-    file_provider._download_artifact(file_provider.artifacts[0], guest, Path("/remote/foo.rpm"))
+    file_provider._download_artifact(
+        file_provider.artifacts[0],
+        guest,
+        Path("/remote/foo-1.0-1.fc43.x86_64.rpm"),
+    )
     guest.push.assert_called_once()
 
     # Test URL download
-    url_provider = artifact_provider("file:https://example.com/foo.rpm")
-    url_provider._download_artifact(url_provider.artifacts[0], guest, Path("/remote/foo.rpm"))
+    url_provider = artifact_provider("file:https://example.com/foo-1.0-1.fc43.x86_64.rpm")
+    url_provider._download_artifact(
+        url_provider.artifacts[0],
+        guest,
+        Path("/remote/foo-1.0-1.fc43.x86_64.rpm"),
+    )
     guest.execute.assert_called_once()
+
+
+@pytest.mark.parametrize(
+    ("filename", "expected"),
+    [
+        (
+            "bash-5.1.8-6.el9.x86_64.rpm",
+            {
+                "name": "bash",
+                "epoch": 0,
+                "version": "5.1.8",
+                "release": "6.el9",
+                "arch": "x86_64",
+            },
+        ),
+        (
+            "tmt+export-polarion-1.61.0.dev17+gf29b2e83e-1.fc41.x86_64.rpm",
+            {
+                "name": "tmt+export-polarion",
+                "epoch": 0,
+                "version": "1.61.0.dev17+gf29b2e83e",
+                "release": "1.fc41",
+                "arch": "x86_64",
+            },
+        ),
+    ],
+)
+def test_version_from_filename_parsing(filename, expected):
+    version = RpmVersion.from_filename(filename)
+
+    assert vars(version) == expected

tests/unit/artifact/test_koji.py

@@ -1,5 +1,6 @@
 import pytest
 
+from tmt.steps.prepare.artifact.providers import RpmVersion
 from tmt.steps.prepare.artifact.providers.koji import (
     KojiArtifactProvider,
     KojiBuild,
@@ -68,3 +69,64 @@ def test_koji_valid_task_id_scratch_build(mock_koji, mock_call_api, artifact_pro
     assert task_id in tasks
     assert provider.build_id is None
     assert len(provider.artifacts) == 2
+
+
+@pytest.mark.parametrize(
+    ("rpm_meta", "expected"),
+    [
+        (
+            {"name": "bash", "version": "5.1.8", "release": "6.el9", "arch": "x86_64"},
+            {
+                "name": "bash",
+                "epoch": 0,
+                "version": "5.1.8",
+                "release": "6.el9",
+                "arch": "x86_64",
+                "nvra": "bash-5.1.8-6.el9.x86_64",
+            },
+        ),
+        (
+            {
+                "name": "docker-ce",
+                "version": "20.10.7",
+                "release": "3.el8",
+                "arch": "x86_64",
+                "epoch": 1,
+            },
+            {
+                "name": "docker-ce",
+                "epoch": 1,
+                "version": "20.10.7",
+                "release": "3.el8",
+                "arch": "x86_64",
+                "nvra": "docker-ce-20.10.7-3.el8.x86_64",
+            },
+        ),
+        (
+            {
+                "name": "tmt+export-polarion",
+                "version": "1.61.0.dev17+gf29b2e83e",
+                "release": "1.fc41",
+                "arch": "x86_64",
+            },
+            {
+                "name": "tmt+export-polarion",
+                "epoch": 0,
+                "version": "1.61.0.dev17+gf29b2e83e",
+                "release": "1.fc41",
+                "arch": "x86_64",
+                "nvra": "tmt+export-polarion-1.61.0.dev17+gf29b2e83e-1.fc41.x86_64",
+            },
+        ),
+    ],
+)
+def test_version_from_rpm_meta_parsing(rpm_meta, expected):
+    version = RpmVersion.from_rpm_meta(rpm_meta)
+
+    assert version.name == expected["name"]
+    assert version.epoch == expected["epoch"]
+    assert version.version == expected["version"]
+    assert version.release == expected["release"]
+    assert version.arch == expected["arch"]
+    assert version.nvra == expected["nvra"]
+    assert str(version) == expected["nvra"]