From 919e33afb3c15c54138150a65dad486305549d08 Mon Sep 17 00:00:00 2001 From: Mathieu Kniewallner Date: Tue, 29 Oct 2024 18:19:34 +0100 Subject: [PATCH] feat(python_file_finder): improve ignore handling (#908) * test(functional): more tests for gitignore * test(python_file_finder): add tests for gitignore * feat(python_file_finder): only use gitignore files when using git * test(functional): use `.ignore` in non-git projects --- src/python_file_finder.rs | 3 +- .../pep_621_project/{.gitignore => .ignore} | 0 .../{.gitignore => .ignore} | 0 .../project_with_gitignore/.gitignore | 2 + .../project_with_gitignore/src/.gitignore | 2 + .../project_with_gitignore/src/barfoo.py | 1 + .../project_with_gitignore/src/baz.py | 1 + .../project_with_src_directory/.gitignore | 1 - .../project_with_src_directory/.ignore | 1 + ..._gitignored.py => this_file_is_ignored.py} | 0 tests/functional/cli/test_cli_gitignore.py | 87 +++++++++++++- tests/unit/test_python_file_finder.py | 107 ++++++++++++++++++ 12 files changed, 200 insertions(+), 5 deletions(-) rename tests/fixtures/pep_621_project/{.gitignore => .ignore} (100%) rename tests/fixtures/project_using_namespace/{.gitignore => .ignore} (100%) create mode 100644 tests/fixtures/project_with_gitignore/src/.gitignore create mode 100644 tests/fixtures/project_with_gitignore/src/barfoo.py create mode 100644 tests/fixtures/project_with_gitignore/src/baz.py delete mode 100644 tests/fixtures/project_with_src_directory/.gitignore create mode 100644 tests/fixtures/project_with_src_directory/.ignore rename tests/fixtures/project_with_src_directory/src/{this_file_is_gitignored.py => this_file_is_ignored.py} (100%) diff --git a/src/python_file_finder.rs b/src/python_file_finder.rs index 493df2a0..f3d909d0 100644 --- a/src/python_file_finder.rs +++ b/src/python_file_finder.rs @@ -61,9 +61,8 @@ fn build_walker( walk_builder .types(build_types(ignore_notebooks).unwrap()) + .standard_filters(use_git_ignore) .hidden(false) - .git_ignore(use_git_ignore) - .require_git(false) .filter_entry(move |entry| entry_satisfies_predicate(entry, re.as_ref())) .build() } diff --git a/tests/fixtures/pep_621_project/.gitignore b/tests/fixtures/pep_621_project/.ignore similarity index 100% rename from tests/fixtures/pep_621_project/.gitignore rename to tests/fixtures/pep_621_project/.ignore diff --git a/tests/fixtures/project_using_namespace/.gitignore b/tests/fixtures/project_using_namespace/.ignore similarity index 100% rename from tests/fixtures/project_using_namespace/.gitignore rename to tests/fixtures/project_using_namespace/.ignore diff --git a/tests/fixtures/project_with_gitignore/.gitignore b/tests/fixtures/project_with_gitignore/.gitignore index 044d1c59..24837bd2 100644 --- a/tests/fixtures/project_with_gitignore/.gitignore +++ b/tests/fixtures/project_with_gitignore/.gitignore @@ -1 +1,3 @@ +/build foobar.py +/src/barfoo.py diff --git a/tests/fixtures/project_with_gitignore/src/.gitignore b/tests/fixtures/project_with_gitignore/src/.gitignore new file mode 100644 index 00000000..ac77925d --- /dev/null +++ b/tests/fixtures/project_with_gitignore/src/.gitignore @@ -0,0 +1,2 @@ +/baz.py +/src/bar.py diff --git a/tests/fixtures/project_with_gitignore/src/barfoo.py b/tests/fixtures/project_with_gitignore/src/barfoo.py new file mode 100644 index 00000000..88af17dd --- /dev/null +++ b/tests/fixtures/project_with_gitignore/src/barfoo.py @@ -0,0 +1 @@ +import hello diff --git a/tests/fixtures/project_with_gitignore/src/baz.py b/tests/fixtures/project_with_gitignore/src/baz.py new file mode 100644 index 00000000..b05e2854 --- /dev/null +++ b/tests/fixtures/project_with_gitignore/src/baz.py @@ -0,0 +1 @@ +import hej diff --git a/tests/fixtures/project_with_src_directory/.gitignore b/tests/fixtures/project_with_src_directory/.gitignore deleted file mode 100644 index 891399da..00000000 --- a/tests/fixtures/project_with_src_directory/.gitignore +++ /dev/null @@ -1 +0,0 @@ -src/this_file_is_gitignored.py diff --git a/tests/fixtures/project_with_src_directory/.ignore b/tests/fixtures/project_with_src_directory/.ignore new file mode 100644 index 00000000..c9dd1c7d --- /dev/null +++ b/tests/fixtures/project_with_src_directory/.ignore @@ -0,0 +1 @@ +src/this_file_is_ignored.py diff --git a/tests/fixtures/project_with_src_directory/src/this_file_is_gitignored.py b/tests/fixtures/project_with_src_directory/src/this_file_is_ignored.py similarity index 100% rename from tests/fixtures/project_with_src_directory/src/this_file_is_gitignored.py rename to tests/fixtures/project_with_src_directory/src/this_file_is_ignored.py diff --git a/tests/functional/cli/test_cli_gitignore.py b/tests/functional/cli/test_cli_gitignore.py index 9e815a33..ce78d064 100644 --- a/tests/functional/cli/test_cli_gitignore.py +++ b/tests/functional/cli/test_cli_gitignore.py @@ -14,9 +14,13 @@ @pytest.mark.xdist_group(name=Project.GITIGNORE) -def test_cli_gitignore_is_used(pip_venv_factory: PipVenvFactory) -> None: +def test_cli_gitignore_used(pip_venv_factory: PipVenvFactory) -> None: with pip_venv_factory(Project.GITIGNORE) as virtual_env: issue_report = f"{uuid.uuid4()}.json" + + # Simulate the fact that the project is a git repository. + Path(".git").mkdir(exist_ok=True) + result = virtual_env.run(f"deptry . -o {issue_report}") assert result.returncode == 1 @@ -61,9 +65,64 @@ def test_cli_gitignore_is_used(pip_venv_factory: PipVenvFactory) -> None: @pytest.mark.xdist_group(name=Project.GITIGNORE) -def test_cli_gitignore_is_not_used(pip_venv_factory: PipVenvFactory) -> None: +def test_cli_gitignore_used_for_non_root_directory(pip_venv_factory: PipVenvFactory) -> None: with pip_venv_factory(Project.GITIGNORE) as virtual_env: issue_report = f"{uuid.uuid4()}.json" + + # Simulate the fact that the project is a git repository. + Path(".git").mkdir(exist_ok=True) + + result = virtual_env.run(f"deptry src -o {issue_report}") + + assert result.returncode == 1 + assert get_issues_report(Path(issue_report)) == [ + { + "error": { + "code": "DEP002", + "message": "'requests' defined as a dependency but not used in the codebase", + }, + "module": "requests", + "location": { + "file": str(Path("pyproject.toml")), + "line": None, + "column": None, + }, + }, + { + "error": { + "code": "DEP002", + "message": "'mypy' defined as a dependency but not used in the codebase", + }, + "module": "mypy", + "location": { + "file": str(Path("pyproject.toml")), + "line": None, + "column": None, + }, + }, + { + "error": { + "code": "DEP002", + "message": "'pytest' defined as a dependency but not used in the codebase", + }, + "module": "pytest", + "location": { + "file": str(Path("pyproject.toml")), + "line": None, + "column": None, + }, + }, + ] + + +@pytest.mark.xdist_group(name=Project.GITIGNORE) +def test_cli_gitignore_not_used_when_using_exclude(pip_venv_factory: PipVenvFactory) -> None: + with pip_venv_factory(Project.GITIGNORE) as virtual_env: + issue_report = f"{uuid.uuid4()}.json" + + # Simulate the fact that the project is a git repository. + Path(".git").mkdir(exist_ok=True) + result = virtual_env.run(f"deptry . --exclude build/|src/bar.py -o {issue_report}") assert result.returncode == 1 @@ -104,4 +163,28 @@ def test_cli_gitignore_is_not_used(pip_venv_factory: PipVenvFactory) -> None: "column": None, }, }, + { + "error": { + "code": "DEP001", + "message": "'hello' imported but missing from the dependency definitions", + }, + "module": "hello", + "location": { + "file": str(Path("src/barfoo.py")), + "line": 1, + "column": 8, + }, + }, + { + "error": { + "code": "DEP001", + "message": "'hej' imported but missing from the dependency definitions", + }, + "module": "hej", + "location": { + "file": str(Path("src/baz.py")), + "line": 1, + "column": 8, + }, + }, ] diff --git a/tests/unit/test_python_file_finder.py b/tests/unit/test_python_file_finder.py index d777c035..6b0af65a 100644 --- a/tests/unit/test_python_file_finder.py +++ b/tests/unit/test_python_file_finder.py @@ -174,3 +174,110 @@ def test_duplicates_are_removed(tmp_path: Path) -> None: files = get_all_python_files_in((Path(), Path()), exclude=(), extend_exclude=(), using_default_exclude=False) assert sorted(files) == [Path("dir/subdir/file1.py")] + + +def test_gitignore_used_in_git_project(tmp_path: Path) -> None: + """Test that gitignore files are respected when project uses git.""" + git_project_path = tmp_path / "git_project" + git_project_path.mkdir() + + # Simulate the presence of a gitignore outside the git project, that should not be used. + with (tmp_path / ".gitignore").open("w") as f: + f.write("*") + + with run_within_dir(tmp_path / git_project_path): + # Simulate the fact that the project is a git repository. + Path(".git").mkdir() + + create_files([ + Path("file1.py"), + Path("file2.py"), + Path("file3.py"), + Path("dir1/file.py"), + Path("dir2/file2.py"), + Path("dir3/file3.py"), + Path("dir3/file4.py"), + ]) + + with Path(".gitignore").open("w") as f: + f.write("""/file1.py +/dir1/file.py +file2.py""") + + with Path("dir3/.gitignore").open("w") as f: + f.write("/file3.py") + + files = get_all_python_files_in((Path(),), exclude=(), extend_exclude=(), using_default_exclude=True) + + assert sorted(files) == [ + Path("dir3/file4.py"), + Path("file3.py"), + ] + + +def test_gitignore_ignored_when_not_using_default_exclude(tmp_path: Path) -> None: + """Test that gitignore files are ignored when project uses git but does not use default exclude.""" + git_project_path = tmp_path / "git_project" + git_project_path.mkdir() + + # Simulate the presence of a gitignore outside the git project, that should not be used. + with (tmp_path / ".gitignore").open("w") as f: + f.write("*") + + with run_within_dir(tmp_path / git_project_path): + # Simulate the fact that the project is a git repository. + Path(".git").mkdir() + + create_files([ + Path("file1.py"), + Path("file2.py"), + Path("file3.py"), + Path("dir1/file.py"), + Path("dir2/file2.py"), + Path("dir3/file3.py"), + Path("dir3/file4.py"), + ]) + + with Path(".gitignore").open("w") as f: + f.write("""/file1.py +/dir1/file.py +file2.py""") + + with Path("dir3/.gitignore").open("w") as f: + f.write("/file3.py") + + files = get_all_python_files_in( + (Path(),), + exclude=("file3.py",), + extend_exclude=(), + using_default_exclude=False, + ) + + assert sorted(files) == [ + Path("dir1/file.py"), + Path("dir2/file2.py"), + Path("dir3/file3.py"), + Path("dir3/file4.py"), + Path("file1.py"), + Path("file2.py"), + ] + + +def test_gitignore_ignored_in_non_git_project(tmp_path: Path) -> None: + """Test that gitignore files are ignored when project does not use git.""" + with run_within_dir(tmp_path): + create_files([ + Path("file1.py"), + Path("file2.py"), + ]) + + # This will be ignored, since project is not a git project. + with Path(".gitignore").open("w") as f: + f.write("/file1.py") + + files = get_all_python_files_in((Path(),), exclude=(), extend_exclude=(), using_default_exclude=True) + + assert sorted(files) == [ + Path("file1.py"), + Path("file2.py"), + ]