Skip to content

Commit

Permalink
Make paths system-agnostic
Browse files Browse the repository at this point in the history 
Make all paths in the code system-agnostic. They are either build with os.path.join() or with operations on `Path` object.
  • Loading branch information
@mknorps
mknorps committed Dec 21, 2023
1 parent 03fcd44 commit 5843d1e
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 86 deletions.
7 changes: 4 additions & 3 deletions tests/project_helpers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def get(self, cache: pytest.Cache) -> Path:

@property
def cache_key(self) -> str:
return f"fawltydeps/{self.tarball_name()}"
return os.path.join("fawltydeps", f"{self.tarball_name()}")

def tarball_path(self, cache: pytest.Cache) -> Path:
return self.cache_dir(cache) / self.tarball_name()
Expand Down Expand Up @@ -166,22 +166,23 @@ def __call__(self, cache: pytest.Cache) -> Path:
script or the requirements to create that venv change.
"""
# We cache venv dirs using the hash from create_venv_hash
cached_str = cache.get(f"fawltydeps/{self.venv_hash()}", None)
cached_str = cache.get(os.path.join("fawltydeps", f"{self.venv_hash()}"), None)
if cached_str is not None and Path(cached_str, ".installed").is_file():
return Path(cached_str) # already cached

# Must run the script to set up the venv
venv_dir = Path(cache.mkdir(f"fawltydeps_venv_{self.venv_hash()}"))
logger.info(f"Creating venv at {venv_dir}...")
venv_script = self.venv_script_lines(venv_dir)

subprocess.run(
" && ".join(venv_script),
check=True, # fail if any of the commands fail
shell=True, # pass multiple shell commands to the subprocess
)
# Make sure the venv has been installed
assert (venv_dir / ".installed").is_file()
cache.set(f"fawltydeps/{self.venv_hash()}", str(venv_dir))
cache.set(os.path.join("fawltydeps", f"{self.venv_hash()}"), str(venv_dir))
return venv_dir


Expand Down
116 changes: 66 additions & 50 deletions tests/test_cmdline.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

import json
import logging
import os
import platform
import sys
from dataclasses import dataclass, field
from itertools import dropwhile
Expand Down Expand Up @@ -111,11 +113,11 @@ def test_list_imports__from_py_file__prints_imports_from_file(write_tmp_files):
)

expect = [
f"{tmp_path}/myfile.py:{n}: {i}"
f"{tmp_path / 'myfile.py'}:{n}: {i}"
for i, n in [("requests", 4), ("foo", 5), ("numpy", 6)]
]
output, returncode = run_fawltydeps_function(
"--list-imports", "--detailed", f"--code={tmp_path}/myfile.py"
"--list-imports", "--detailed", f"--code={tmp_path / 'myfile.py'}"
)
assert output.splitlines() == expect
assert returncode == 0
Expand All @@ -138,23 +140,29 @@ def test_list_imports_json__from_py_file__prints_imports_from_file(write_tmp_fil
expect = {
"settings": make_json_settings_dict(
actions=["list_imports"],
code=[f"{tmp_path}/myfile.py"],
code=[f"{tmp_path / 'myfile.py'}"],
output_format="json",
),
"sources": [
{
"source_type": "CodeSource",
"path": f"{tmp_path}/myfile.py",
"path": f"{tmp_path / 'myfile.py'}",
"base_dir": None,
},
],
"imports": [
{
"name": "requests",
"source": {"path": f"{tmp_path}/myfile.py", "lineno": 4},
"source": {"path": f"{tmp_path / 'myfile.py'}", "lineno": 4},
},
{
"name": "foo",
"source": {"path": f"{tmp_path / 'myfile.py'}", "lineno": 5},
},
{
"name": "numpy",
"source": {"path": f"{tmp_path / 'myfile.py'}", "lineno": 6},
},
{"name": "foo", "source": {"path": f"{tmp_path}/myfile.py", "lineno": 5}},
{"name": "numpy", "source": {"path": f"{tmp_path}/myfile.py", "lineno": 6}},
],
"declared_deps": None,
"resolved_deps": None,
Expand All @@ -163,7 +171,7 @@ def test_list_imports_json__from_py_file__prints_imports_from_file(write_tmp_fil
"version": version(),
}
output, returncode = run_fawltydeps_function(
"--list-imports", "--json", f"--code={tmp_path}/myfile.py"
"--list-imports", "--json", f"--code={tmp_path / 'myfile.py'}"
)
assert json.loads(output) == expect
assert returncode == 0
Expand All @@ -176,9 +184,9 @@ def test_list_imports__from_ipynb_file__prints_imports_from_file(write_tmp_files
}
)

expect = [f"{tmp_path}/myfile.ipynb[1]:1: pytorch"]
expect = [f"{tmp_path / 'myfile.ipynb'}[1]:1: pytorch"]
output, returncode = run_fawltydeps_function(
"--list-imports", "--detailed", f"--code={tmp_path}/myfile.ipynb"
"--list-imports", "--detailed", f"--code={tmp_path / 'myfile.ipynb'}"
)
assert output.splitlines() == expect
assert returncode == 0
Expand All @@ -204,9 +212,9 @@ def test_list_imports__from_dir__prints_imports_from_py_and_ipynb_files_only(
)

expect = [
f"{tmp_path}/file1.py:{n}: {i}"
f"{tmp_path / 'file1.py'}:{n}: {i}"
for i, n in [("my_pathlib", 1), ("pandas", 2), ("scipy", 2)]
] + [f"{tmp_path}/file3.ipynb[1]:1: pytorch"]
] + [f"{tmp_path / 'file3.ipynb'}[1]:1: pytorch"]
output, returncode = run_fawltydeps_function(
"--list-imports", "--detailed", f"--code={tmp_path}"
)
Expand Down Expand Up @@ -309,8 +317,8 @@ def test_list_deps_detailed__dir__prints_deps_from_requirements_txt(fake_project
)

expect = [
f"{tmp_path}/requirements.txt: pandas",
f"{tmp_path}/requirements.txt: requests",
f"{tmp_path / 'requirements.txt'}: pandas",
f"{tmp_path / 'requirements.txt'}: requests",
]
output, returncode = run_fawltydeps_function(
"--list-deps", "--detailed", f"--deps={tmp_path}"
Expand All @@ -332,19 +340,19 @@ def test_list_deps_json__dir__prints_deps_from_requirements_txt(fake_project):
"sources": [
{
"source_type": "DepsSource",
"path": f"{tmp_path}/requirements.txt",
"path": f"{tmp_path / 'requirements.txt'}",
"parser_choice": "requirements.txt",
},
],
"imports": None,
"declared_deps": [
{
"name": "requests",
"source": {"path": f"{tmp_path}/requirements.txt"},
"source": {"path": f"{tmp_path / 'requirements.txt'}"},
},
{
"name": "pandas",
"source": {"path": f"{tmp_path}/requirements.txt"},
"source": {"path": f"{tmp_path / 'requirements.txt'}"},
},
],
"resolved_deps": None,
Expand Down Expand Up @@ -429,8 +437,8 @@ def test_list_sources__in_varied_project__lists_all_files(fake_project):
tmp_path = fake_project(
files_with_imports={
"code.py": ["foo"],
"subdir/other.py": ["foo"],
"subdir/notebook.ipynb": ["foo"],
os.path.join("subdir", "other.py"): ["foo"],
os.path.join("subdir", "notebook.ipynb"): ["foo"],
},
files_with_declared_deps={
"requirements.txt": ["foo"],
Expand All @@ -446,8 +454,8 @@ def test_list_sources__in_varied_project__lists_all_files(fake_project):
str(tmp_path / filename)
for filename in [
"code.py",
"subdir/other.py",
"subdir/notebook.ipynb",
os.path.join("subdir", "other.py"),
os.path.join("subdir", "notebook.ipynb"),
"requirements.txt",
"pyproject.toml",
"setup.py",
Expand All @@ -465,8 +473,8 @@ def test_list_sources_detailed__in_varied_project__lists_all_files(fake_project)
tmp_path = fake_project(
files_with_imports={
"code.py": ["foo"],
"subdir/notebook.ipynb": ["foo"],
"subdir/other.py": ["foo"],
os.path.join("subdir", "notebook.ipynb"): ["foo"],
os.path.join("subdir", "other.py"): ["foo"],
},
files_with_declared_deps={
"pyproject.toml": ["foo"],
Expand All @@ -477,19 +485,19 @@ def test_list_sources_detailed__in_varied_project__lists_all_files(fake_project)
fake_venvs={"my_venv": {}},
)
output, returncode = run_fawltydeps_function(
"--list-sources", f"{tmp_path}", "--detailed"
"--list-sources", str(tmp_path), "--detailed"
)
expect_code_lines = [
f" {tmp_path / filename} (using {tmp_path}/ as base for 1st-party imports)"
f" {tmp_path / filename} (using {tmp_path} as base for 1st-party imports)"
for filename in [
"code.py",
"setup.py", # This is both a CodeSource and an DepsSource!
"subdir/notebook.ipynb",
"subdir/other.py",
os.path.join("subdir", "notebook.ipynb"),
os.path.join("subdir", "other.py"),
]
]
expect_deps_lines = [
f" {tmp_path / filename} (parsed as a {filename} file)"
f" { tmp_path / filename} (parsed as a {filename} file)"
for filename in [
"pyproject.toml",
"requirements.txt",
Expand Down Expand Up @@ -533,9 +541,16 @@ def test_list_sources_detailed__from_both_python_file_and_stdin(fake_project):
"--list-sources", f"{tmp_path}", "--code", f"{tmp_path}", "-", "--detailed"
)
expect = [
"Sources of Python code:",
f" {tmp_path}/code.py (using {tmp_path}/ as base for 1st-party imports)",
" <stdin>",
[
"Sources of Python code:",
f" {tmp_path / 'code.py'} (using {tmp_path} as base for 1st-party imports)",
" <stdin>",
],
[
"Sources of Python code:",
" <stdin>",
f" {tmp_path / 'code.py'} (using {tmp_path} as base for 1st-party imports)",
],
]
assert output.splitlines() == expect
assert returncode == 0
Expand Down Expand Up @@ -575,7 +590,7 @@ class ProjectTestVector:
expect_output=[
"These imports appear to be undeclared dependencies:",
"- 'requests' imported at:",
" {path}/code.py:1",
f" {os.path.join('{path}', 'code.py')}:1",
],
expect_returncode=3,
),
Expand All @@ -587,7 +602,7 @@ class ProjectTestVector:
expect_output=[
"These dependencies appear to be unused (i.e. not imported):",
"- 'pandas' declared in:",
" {path}/requirements.txt",
f" {os.path.join('{path}', 'requirements.txt')}",
],
expect_returncode=4,
),
Expand All @@ -599,11 +614,11 @@ class ProjectTestVector:
expect_output=[
"These imports appear to be undeclared dependencies:",
"- 'requests' imported at:",
" {path}/code.py:1",
f" {os.path.join('{path}', 'code.py')}:1",
"",
"These dependencies appear to be unused (i.e. not imported):",
"- 'pandas' declared in:",
" {path}/requirements.txt",
f" {os.path.join('{path}', 'requirements.txt')}",
],
expect_returncode=3, # undeclared is more important than unused
),
Expand All @@ -623,7 +638,8 @@ class ProjectTestVector:
],
expect_logs=[
"INFO:fawltydeps.extract_imports:Finding Python files under {path}",
"INFO:fawltydeps.extract_imports:Parsing Python file {path}/code.py",
"INFO:fawltydeps.extract_imports:Parsing Python file "
f"{os.path.join('{path}', 'code.py')}",
"INFO:fawltydeps.packages:'pandas' was not resolved."
" Assuming it can be imported as 'pandas'.",
],
Expand All @@ -637,11 +653,11 @@ class ProjectTestVector:
expect_output=[
"These imports appear to be undeclared dependencies:",
"- 'requests' imported at:",
" {path}/code.py:1",
f" {os.path.join('{path}', 'code.py')}:1",
"",
"These dependencies appear to be unused (i.e. not imported):",
"- 'pandas' declared in:",
" {path}/requirements.txt",
f" {os.path.join('{path}', 'requirements.txt')}",
],
expect_returncode=3, # undeclared is more important than unused
),
Expand Down Expand Up @@ -691,12 +707,12 @@ def test_check_json__simple_project__can_report_both_undeclared_and_unused(
"sources": [
{
"source_type": "CodeSource",
"path": f"{tmp_path}/code.py",
"path": f"{tmp_path / 'code.py'}",
"base_dir": f"{tmp_path}",
},
{
"source_type": "DepsSource",
"path": f"{tmp_path}/requirements.txt",
"path": f"{tmp_path / 'requirements.txt'}",
"parser_choice": "requirements.txt",
},
{
Expand All @@ -711,13 +727,13 @@ def test_check_json__simple_project__can_report_both_undeclared_and_unused(
"imports": [
{
"name": "requests",
"source": {"path": f"{tmp_path}/code.py", "lineno": 1},
"source": {"path": f"{tmp_path / 'code.py'}", "lineno": 1},
},
],
"declared_deps": [
{
"name": "pandas",
"source": {"path": f"{tmp_path}/requirements.txt"},
"source": {"path": f"{tmp_path / 'requirements.txt'}"},
},
],
"resolved_deps": {
Expand All @@ -731,13 +747,13 @@ def test_check_json__simple_project__can_report_both_undeclared_and_unused(
"undeclared_deps": [
{
"name": "requests",
"references": [{"path": f"{tmp_path}/code.py", "lineno": 1}],
"references": [{"path": f"{tmp_path / 'code.py'}", "lineno": 1}],
},
],
"unused_deps": [
{
"name": "pandas",
"references": [{"path": f"{tmp_path}/requirements.txt"}],
"references": [{"path": f"{tmp_path / 'requirements.txt'}"}],
},
],
"version": version(),
Expand Down Expand Up @@ -932,34 +948,34 @@ def test_check_json__no_pyenvs_found__falls_back_to_current_env(fake_project):
"sources": [
{
"source_type": "CodeSource",
"path": f"{tmp_path}/code.py",
"path": f"{tmp_path / 'code.py'}",
"base_dir": f"{tmp_path}",
},
{
"source_type": "DepsSource",
"path": f"{tmp_path}/requirements.txt",
"path": f"{tmp_path / 'requirements.txt'}",
"parser_choice": "requirements.txt",
},
# No PyEnvSources found
],
"imports": [
{
"name": "packaging_legacy_version",
"source": {"path": f"{tmp_path}/code.py", "lineno": 1},
"source": {"path": f"{tmp_path / 'code.py'}", "lineno": 1},
},
{
"name": "other_module",
"source": {"path": f"{tmp_path}/code.py", "lineno": 2},
"source": {"path": f"{tmp_path / 'code.py'}", "lineno": 2},
},
],
"declared_deps": [
{
"name": "pip-requirements-parser",
"source": {"path": f"{tmp_path}/requirements.txt"},
"source": {"path": f"{tmp_path / 'requirements.txt'}"},
},
{
"name": "other_module",
"source": {"path": f"{tmp_path}/requirements.txt"},
"source": {"path": f"{tmp_path / 'requirements.txt'}"},
},
],
"resolved_deps": {
Expand Down
Loading

0 comments on commit 5843d1e

Please sign in to comment.