Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix converting of editable package #288

Merged
merged 16 commits into from
Aug 2, 2023
Merged
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ source = ["src"]
omit = [
"src/npe2/manifest/contributions/_keybindings.py",
"src/npe2/manifest/menus.py",
"src/npe2/__main__.py",
"src/npe2/manifest/package_metadata.py",
# due to all of the isolated sub-environments and sub-processes,
# it's really hard to get coverage on the setuptools plugin.
Expand Down
4 changes: 4 additions & 0 deletions src/npe2/__main__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
from npe2.cli import main

if __name__ == "__main__":
main()
8 changes: 7 additions & 1 deletion src/npe2/_inspection/_from_npe1.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,13 @@ def get_top_module_path(package_name, top_module: Optional[str] = None) -> Path:
top_module = top_mods[0]

path = Path(dist.locate_file(top_module))
assert path.is_dir()
if not path.is_dir() and dist.files:
for f_path in dist.files:
if "__editable__" in f_path.name:
path = Path(f_path.read_text().strip()) / top_module
break
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Czaki I have zero context here so take my review with a grain of salt, but if there are no tests here, could you at least add comments about why these lines are needed? eg "if the given npe1 plugin is installed in editable mode, then X doesn't work so we must do Y"


assert path.is_dir(), f"Could not find top level module {top_module} using {path}"
return path


Expand Down
47 changes: 47 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,53 @@ def _from_name(name):
(npe1_repo / "setup.py").unlink()


@pytest.fixture
def mock_npe1_pm_with_plugin_editable(npe1_repo, npe1_plugin_module, tmp_path):
"""Mocks a fully installed local repository"""
from npe2._inspection._from_npe1 import plugin_packages

dist_path = tmp_path / "npe1-plugin-0.0.1.dist-info"
shutil.copytree(npe1_repo / "npe1-plugin-0.0.1.dist-info", dist_path)

record_path = dist_path / "RECORD"

record_content = record_path.read_text().splitlines()
record_content.pop(-1)
record_content.append("__editable__.npe1-plugin-0.0.1.pth")

with record_path.open("w") as f:
f.write("\n".join(record_content))

with open(tmp_path / "__editable__.npe1-plugin-0.0.1.pth", "w") as f:
f.write(str(npe1_repo))

mock_dist = metadata.PathDistribution(dist_path)

def _dists():
return [mock_dist]

def _from_name(name):
if name == "npe1-plugin":
return mock_dist
raise metadata.PackageNotFoundError(name)

setup_cfg = npe1_repo / "setup.cfg"
new_manifest = npe1_repo / "npe1_module" / "napari.yaml"
with patch.object(metadata, "distributions", new=_dists):
with patch.object(metadata.Distribution, "from_name", new=_from_name):
cfg = setup_cfg.read_text()
plugin_packages.cache_clear()
try:
yield mock_npe1_pm
finally:
plugin_packages.cache_clear()
setup_cfg.write_text(cfg)
if new_manifest.exists():
new_manifest.unlink()
if (npe1_repo / "setup.py").exists():
(npe1_repo / "setup.py").unlink()


@pytest.fixture(autouse=True)
def mock_cache(tmp_path, monkeypatch):
with monkeypatch.context() as m:
Expand Down
5 changes: 5 additions & 0 deletions tests/npe1-plugin/npe1-plugin-0.0.1.dist-info/RECORD
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
npe1-plugin-0.0.1.dist-info/RECORD,,
npe1-plugin-0.0.1.dist-info/METADATA,,
npe1-plugin-0.0.1.dist-info/top_level.txt,,
npe1-plugin-0.0.1.dist-info/entry_points.txt,,
../npe1_module/__init__.py,,
24 changes: 24 additions & 0 deletions tests/test_conversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,30 @@ def test_conversion_from_package(npe1_repo, mock_npe1_pm_with_plugin):
assert "Is this package already converted?" in str(e.value)


@pytest.mark.filterwarnings("ignore:Failed to convert napari_provide_sample_data")
@pytest.mark.filterwarnings("ignore:Error converting function")
@pytest.mark.filterwarnings("ignore:Error converting dock widget")
def test_conversion_from_package_editable(npe1_repo, mock_npe1_pm_with_plugin_editable):
setup_cfg = npe1_repo / "setup.cfg"
before = setup_cfg.read_text()
convert_repository(npe1_repo, dry_run=True)
assert setup_cfg.read_text() == before
assert not (npe1_repo / "npe1_module" / "napari.yaml").exists()
convert_repository(npe1_repo, dry_run=False)
new_setup = setup_cfg.read_text()
assert new_setup != before
assert (
"[options.entry_points]\n"
"napari.manifest = \n npe1-plugin = npe1_module:napari.yaml"
) in new_setup
assert "[options.package_data]\nnpe1_module = napari.yaml" in new_setup
assert (npe1_repo / "npe1_module" / "napari.yaml").is_file()

with pytest.raises(ValueError) as e:
convert_repository(npe1_repo)
assert "Is this package already converted?" in str(e.value)


def _assert_expected_errors(record: pytest.WarningsRecorder):
assert len(record) == 4
msg = str(record[0].message)
Expand Down