Skip to content

Commit

Permalink
Correctly detect transitive dependencies with different module names (#…
Browse files Browse the repository at this point in the history
…1033)

* modify the problem transitive dependency being detected as missing wrongl

Added another check method to verify whether a package exists in the environment, if can't be found by _get_package_name_from_metadata.
Also updated the test cases to include bs4 for testing in test_cli_requirement_in.

* Refs: #1099 revise the check function and add dependency into requirements.in

I revised module.py as per the change requests and added beautifulsoup4==4.12.3 to requirements.in. With this change, the test_cli_single_requirements_files test now passes without any DEP003 issues related to bs4. Additionally, since soupsieve is added to requirements.txt during the uvx --from pip-tools pip-compile process as a dependency of bs4, this causes a DEP002 issue in test_cli_multiple_requirements_files. Therefore, I also revised these two parts of the test case.

* run through pre-commit

* add a unittest and revise the functional test related to #1099 pull request

Add a unit test to test the new function _is_package_installed in module.py, also revise the test case in project_requirements_in to verify the use of the new function.

* Update tests/fixtures/project_with_requirements_in/requirements.txt

Co-authored-by: Mathieu Kniewallner <[email protected]>

* Update test_module.py

---------

Co-authored-by: Mathieu Kniewallner <[email protected]>
  • Loading branch information
Willa1228 and mkniewallner authored Jan 25, 2025
1 parent a5d01d8 commit ddbb004
Show file tree
Hide file tree
Showing 6 changed files with 127 additions and 4 deletions.
9 changes: 8 additions & 1 deletion python/deptry/module.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import logging
from dataclasses import dataclass, field
from importlib.metadata import PackageNotFoundError, metadata
from importlib.util import find_spec
from typing import TYPE_CHECKING

if TYPE_CHECKING:
Expand Down Expand Up @@ -118,10 +119,16 @@ def _get_package_name_from_metadata(self) -> str | None:
try:
name: str = metadata(self.name)["Name"]
except PackageNotFoundError:
return None
return self.name if self._is_package_installed() else None
else:
return name

def _is_package_installed(self) -> bool:
try:
return find_spec(self.name) is not None
except (ModuleNotFoundError, ValueError):
return False

def _get_corresponding_top_levels_from(self, dependencies: list[Dependency]) -> list[str]:
"""
Not all modules have associated metadata. e.g. `mpl_toolkits` from `matplotlib` has no metadata. However, it is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ click==8.1.7
isort==5.13.2
urllib3==2.2.3
uvicorn==0.32.0
itchiodl==2.3.0
26 changes: 23 additions & 3 deletions tests/fixtures/project_with_requirements_in/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,35 @@
#
# pip-compile
#
click==8.1.8
args==0.1.0
# via clint
beautifulsoup4==4.12.3
# via itchiodl
certifi==2024.12.14
# via requests
charset-normalizer==3.4.1
# via requests
click==8.1.7
# via
# -r requirements.in
# uvicorn
clint==0.5.1
# via itchiodl
h11==0.14.0
# via uvicorn
idna==3.10
# via requests
isort==5.13.2
# via -r requirements.in
urllib3==2.3.0
itchiodl==2.3.0
# via -r requirements.in
uvicorn==0.34.0
requests==2.32.3
# via itchiodl
soupsieve==2.6
# via beautifulsoup4
urllib3==2.2.3
# via
# -r requirements.in
# requests
uvicorn==0.32.0
# via -r requirements.in
2 changes: 2 additions & 0 deletions tests/fixtures/project_with_requirements_in/src/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,5 @@
import h11
import white as w
from urllib3 import contrib
import bs4
import itchiodl
52 changes: 52 additions & 0 deletions tests/functional/cli/test_cli_requirements_in.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ def test_cli_single_requirements_files(pip_venv_factory: PipVenvFactory) -> None
"module": "white",
"location": {"file": str(Path("src/main.py")), "line": 7, "column": 8},
},
{
"error": {"code": "DEP003", "message": "'bs4' imported but it is a transitive dependency"},
"module": "bs4",
"location": {"file": str(Path("src/main.py")), "line": 9, "column": 8},
},
{
"error": {"code": "DEP001", "message": "'arrow' imported but missing from the dependency definitions"},
"module": "arrow",
Expand All @@ -80,11 +85,58 @@ def test_cli_multiple_requirements_files(pip_venv_factory: PipVenvFactory) -> No

assert result.returncode == 1
assert get_issues_report(Path(issue_report)) == [
{
"error": {"code": "DEP002", "message": "'args' defined as a dependency but not used in the codebase"},
"module": "args",
"location": {"file": str(Path("requirements.txt")), "line": None, "column": None},
},
{
"error": {
"code": "DEP002",
"message": "'certifi' defined as a dependency but not used in the codebase",
},
"module": "certifi",
"location": {"file": str(Path("requirements.txt")), "line": None, "column": None},
},
{
"error": {
"code": "DEP002",
"message": "'charset-normalizer' defined as a dependency but not used in the codebase",
},
"module": "charset-normalizer",
"location": {"file": str(Path("requirements.txt")), "line": None, "column": None},
},
{
"error": {"code": "DEP002", "message": "'clint' defined as a dependency but not used in the codebase"},
"module": "clint",
"location": {"file": str(Path("requirements.txt")), "line": None, "column": None},
},
{
"error": {"code": "DEP002", "message": "'idna' defined as a dependency but not used in the codebase"},
"module": "idna",
"location": {"file": str(Path("requirements.txt")), "line": None, "column": None},
},
{
"error": {"code": "DEP002", "message": "'isort' defined as a dependency but not used in the codebase"},
"module": "isort",
"location": {"file": str(Path("requirements.txt")), "line": None, "column": None},
},
{
"error": {
"code": "DEP002",
"message": "'requests' defined as a dependency but not used in the codebase",
},
"module": "requests",
"location": {"file": str(Path("requirements.txt")), "line": None, "column": None},
},
{
"error": {
"code": "DEP002",
"message": "'soupsieve' defined as a dependency but not used in the codebase",
},
"module": "soupsieve",
"location": {"file": "requirements.txt", "line": None, "column": None},
},
{
"error": {
"code": "DEP002",
Expand Down
41 changes: 41 additions & 0 deletions tests/unit/test_module.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
from __future__ import annotations

from importlib.metadata import PackageNotFoundError
from pathlib import Path
from unittest.mock import patch

import pytest

from deptry.dependency import Dependency
from deptry.module import ModuleBuilder
Expand Down Expand Up @@ -35,3 +39,40 @@ def test_local_module() -> None:
assert module.package is None
assert module.standard_library is False
assert module.local_module is True


def test_transitive_module() -> None:
with (
patch("deptry.module.metadata", side_effect=PackageNotFoundError),
patch("deptry.module.find_spec", return_value="bar"),
):
module = ModuleBuilder("foo", set(), frozenset()).build()

assert module.package == "foo"
assert module.standard_library is False
assert module.local_module is False


def test_transitive_module_no_spec() -> None:
with (
patch("deptry.module.metadata", side_effect=PackageNotFoundError),
patch("deptry.module.find_spec", return_value=None),
):
module = ModuleBuilder("foo", set(), frozenset()).build()

assert module.package is None
assert module.standard_library is False
assert module.local_module is False


@pytest.mark.parametrize("exception", [ModuleNotFoundError, ValueError])
def test_transitive_module_spec_error(exception: Exception) -> None:
with (
patch("deptry.module.metadata", side_effect=PackageNotFoundError),
patch("deptry.module.find_spec", side_effect=exception),
):
module = ModuleBuilder("foo", set(), frozenset()).build()

assert module.package is None
assert module.standard_library is False
assert module.local_module is False

0 comments on commit ddbb004

Please sign in to comment.