opam: Move the state initialization into the get_pages method
We should avoid side-effects in the constructor as much as possible. That avoids surprising behavior at object instantiation time. The state if needed must be initialized into the `swh.lister.pattern.Lister.get_pages` method, as preconized in the class docstring. This also fixes the current test that actually bootstrap a real opam local "clone" in /tmp. Related to T3590
This commit is contained in:
parent
c803fc2b59
commit
b69b0b7fd6
2 changed files with 32 additions and 12 deletions
|
@ -51,7 +51,12 @@ class OpamLister(StatelessLister[PageType]):
|
|||
scheduler=scheduler, credentials=credentials, url=url, instance=instance,
|
||||
)
|
||||
self.env = os.environ.copy()
|
||||
# Opam root folder is initialized in the :meth:`get_pages` method as no
|
||||
# side-effect should happen in the constructor to ease instantiation
|
||||
self.opamroot = tempfile.mkdtemp(prefix="swh_opam_lister")
|
||||
|
||||
def get_pages(self) -> Iterator[PageType]:
|
||||
# Initialize the opam root directory with the opam instance data to list.
|
||||
call(
|
||||
[
|
||||
"opam",
|
||||
|
@ -61,13 +66,12 @@ class OpamLister(StatelessLister[PageType]):
|
|||
"--no-setup",
|
||||
"--root",
|
||||
self.opamroot,
|
||||
instance,
|
||||
url,
|
||||
self.instance,
|
||||
self.url,
|
||||
],
|
||||
env=self.env,
|
||||
)
|
||||
|
||||
def get_pages(self) -> Iterator[PageType]:
|
||||
# Actually list opam instance data
|
||||
proc = Popen(
|
||||
[
|
||||
"opam",
|
||||
|
|
|
@ -6,24 +6,41 @@
|
|||
import io
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from swh.lister.opam.lister import OpamLister
|
||||
|
||||
module_name = "swh.lister.opam.lister"
|
||||
|
||||
def test_urls(swh_scheduler, mocker):
|
||||
|
||||
@pytest.fixture
|
||||
def mock_opam(mocker):
|
||||
"""Fixture to bypass the actual opam calls within the test context.
|
||||
|
||||
"""
|
||||
# inhibits the real `subprocess.call` which prepares the required internal opam
|
||||
# state
|
||||
mock_init = mocker.patch(f"{module_name}.call", return_value=None)
|
||||
# replaces the real Popen with a fake one (list origins command)
|
||||
mocked_popen = MagicMock()
|
||||
mocked_popen.stdout = io.BytesIO(b"bar\nbaz\nfoo\n")
|
||||
mock_open = mocker.patch(f"{module_name}.Popen", return_value=mocked_popen)
|
||||
return mock_init, mock_open
|
||||
|
||||
|
||||
def test_urls(swh_scheduler, mock_opam):
|
||||
mock_init, mock_popen = mock_opam
|
||||
|
||||
instance_url = "https://opam.ocaml.org"
|
||||
|
||||
lister = OpamLister(swh_scheduler, url=instance_url, instance="opam")
|
||||
|
||||
mocked_popen = MagicMock()
|
||||
mocked_popen.stdout = io.BytesIO(b"bar\nbaz\nfoo\n")
|
||||
|
||||
# replaces the real Popen with a fake one
|
||||
mocker.patch("swh.lister.opam.lister.Popen", return_value=mocked_popen)
|
||||
|
||||
# call the lister and get all listed origins urls
|
||||
stats = lister.run()
|
||||
|
||||
assert mock_init.called
|
||||
assert mock_popen.called
|
||||
|
||||
assert stats.pages == 3
|
||||
assert stats.origins == 3
|
||||
|
||||
|
@ -41,7 +58,6 @@ def test_urls(swh_scheduler, mocker):
|
|||
|
||||
|
||||
def test_opam_binary(datadir, swh_scheduler):
|
||||
|
||||
instance_url = f"file://{datadir}/fake_opam_repo"
|
||||
|
||||
lister = OpamLister(swh_scheduler, url=instance_url, instance="fake")
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue