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.

Sign up for GitHub

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 path inclusion behavior #224

Merged
merged 1 commit into from
Jan 26, 2024
Merged

Fix path inclusion behavior #224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions integration_tests/base_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def check_dependencies_after(self):

class BaseIntegrationTest(DependencyTestMixin, CleanRepoMixin):
codemod = NotImplementedError
code_path = NotImplementedError
code_path: str = NotImplementedError
original_code = NotImplementedError
expected_new_code = NotImplementedError
output_path = "test-codetf.txt"
Expand Down Expand Up @@ -77,13 +77,14 @@ def setup_method(self):
) from exc

def _assert_run_fields(self, run, output_path):
code_path = os.path.relpath(self.code_path, SAMPLES_DIR)
assert run["vendor"] == "pixee"
assert run["tool"] == "codemodder-python"
assert run["version"] == __version__
assert run["elapsed"] != ""
assert (
run["commandLine"]
== f"codemodder {SAMPLES_DIR} --output {output_path} --codemod-include={self.codemod_instance.name} --path-include={self.code_path}"
== f'codemodder {SAMPLES_DIR} --output {output_path} --codemod-include={self.codemod_instance.name} --path-include={code_path} --path-exclude=""'
)
assert run["directory"] == os.path.abspath(SAMPLES_DIR)
assert run["sarifs"] == []
Expand Down Expand Up @@ -149,14 +150,15 @@ def test_file_rewritten(self):
Mocks won't work when making a subprocess call so make sure to delete all
output files
"""

code_path = os.path.relpath(self.code_path, SAMPLES_DIR)
command = [
"codemodder",
SAMPLES_DIR,
"--output",
self.output_path,
f"--codemod-include={self.codemod_instance.name}",
f"--path-include={self.code_path}",
f"--path-include={code_path}",
'--path-exclude=""',
]

self.check_code_before()
Expand Down
3 changes: 2 additions & 1 deletion integration_tests/test_multiple_codemods.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ def test_two_codemods(self, codemods, tmpdir):
str(codetf_path),
f"--codemod-include={codemods}",
"--path-include",
f"**/{source_file_name}",
f"{source_file_name}",
'--path-exclude=""',
]

completed_process = subprocess.run(
Expand Down
39 changes: 22 additions & 17 deletions src/codemodder/code_directory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,23 @@

DEFAULT_INCLUDED_PATHS = ["**.py", "**/*.py"]
DEFAULT_EXCLUDED_PATHS = [
"**/test/**",
"**/tests/**",
"**/conftest.py",
"**/build/**",
"**/dist/**",
"**/venv/**",
"**/.venv/**",
"**/.tox/**",
"**/.nox/**",
"**/.eggs/**",
"**/.git/**",
"**/.mypy_cache/**",
"**/.pytest_cache/**",
"**/.hypothesis/**",
"**/.coverage*",
# TODO: test code should eventually only be excluded on a per-codemod basis
# Some codemods represent fixes that should be applied to test code
"test/**",
"tests/**",
"conftest.py",
"build/**",
"dist/**",
"venv/**",
".venv/**",
".tox/**",
".nox/**",
".eggs/**",
".git/**",
".mypy_cache/**",
".pytest_cache/**",
".hypothesis/**",
".coverage*",
]


Expand Down Expand Up @@ -64,7 +66,10 @@ def match_files(
:return: list of <pathlib.PosixPath> files found within (including recursively) the parent directory
that match the criteria of both exclude and include patterns.
"""
all_files = [str(path) for path in Path(parent_path).rglob("*")]
all_files = [
str(Path(path).relative_to(parent_path))
for path in Path(parent_path).rglob("*")
]
included_files = set(
filter_files(
all_files,
Expand All @@ -82,5 +87,5 @@ def match_files(
return [
path
for p in sorted(list(included_files - excluded_files))
if (path := Path(p)).is_file() and path.suffix == ".py"
if (path := Path(parent_path).joinpath(p)).is_file() and path.suffix == ".py"
]
1 change: 1 addition & 0 deletions src/codemodder/codemodder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ def run(original_args) -> int:

log_section("startup")
logger.info("codemodder: python/%s", __version__)
logger.info("command: %s %s", Path(sys.argv[0]).name, " ".join(original_args))

repo_manager = PythonRepoManager(Path(argv.directory))
context = CodemodExecutionContext(
Expand Down
18 changes: 9 additions & 9 deletions tests/test_code_directory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def test_all_py_files_match(self, dir_structure):

def test_match_excluded(self, dir_structure):
expected = ["empty_for_testing.py", "insecure_random.py"]
files = match_files(dir_structure, ["**/tests/**", "*request.py"])
Copy link
Contributor

Choose a reason for hiding this comment

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

would it make sense to keep one test with **/tests... ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Possibly but with the new behavior this isn't a valid match anymore.

files = match_files(dir_structure, ["tests/**", "*request.py"])
self._assert_expected(files, expected)

def test_match_included_file_with_line(self, dir_structure):
Expand All @@ -53,7 +53,7 @@ def test_match_included_file_with_line(self, dir_structure):
def test_match_excluded_line(self, dir_structure):
expected = ["empty_for_testing.py", "insecure_random.py", "make_request.py"]
files = match_files(
dir_structure, exclude_paths=["**/tests/**", "**/insecure_random.py:2"]
dir_structure, exclude_paths=["tests/**", "**/insecure_random.py:2"]
)
self._assert_expected(files, expected)

Expand All @@ -65,26 +65,26 @@ def test_match_included_line_and_glob(self, dir_structure):
def test_match_excluded_line_and_glob(self, dir_structure):
expected = ["empty_for_testing.py", "insecure_random.py", "make_request.py"]
files = match_files(
dir_structure, exclude_paths=["**/tests/**", "**/insecure*.py:3"]
dir_structure, exclude_paths=["tests/**", "**/insecure*.py:3"]
)
self._assert_expected(files, expected)

def test_match_excluded_dir_incorrect_glob(self, dir_structure):
incorrect_glob = "more_samples"
expected = ["empty_for_testing.py", "insecure_random.py", "make_request.py"]
files = match_files(dir_structure, ["**/tests/**", incorrect_glob])
files = match_files(dir_structure, ["tests/**", incorrect_glob])
self._assert_expected(files, expected)

def test_match_excluded_dir_correct_glob(self, dir_structure):
correct_globs = ["**/more_samples/**", "*/more_samples/*"]
for correct_glob in correct_globs:
expected = ["insecure_random.py", "make_request.py"]
files = match_files(dir_structure, ["**/tests/**", correct_glob])
files = match_files(dir_structure, ["tests/**", correct_glob])
self._assert_expected(files, expected)

def test_match_excluded_multiple(self, dir_structure):
expected = ["insecure_random.py"]
files = match_files(dir_structure, ["**/tests/**", "*request.py", "*empty*"])
files = match_files(dir_structure, ["tests/**", "*request.py", "*empty*"])
self._assert_expected(files, expected)

def test_match_included(self, dir_structure):
Expand All @@ -96,15 +96,15 @@ def test_match_excluded_precedence_over_included(self, dir_structure):
expected = ["empty_for_testing.py", "insecure_random.py"]
files = match_files(
dir_structure,
exclude_paths=["**/tests/**", "*request.py"],
exclude_paths=["tests/**", "*request.py"],
include_paths=["*request.py", "*empty*.py", "*random.py"],
)
self._assert_expected(files, expected)

def test_test_directory_not_excluded(self, dir_structure):
expected = ["test_insecure_random.py", "test_make_request.py"]
files = match_files(
dir_structure, exclude_paths=["**/samples/**", "**/more_samples/**"]
dir_structure, exclude_paths=["samples/**", "**/more_samples/**"]
)
self._assert_expected(files, expected)

Expand All @@ -120,7 +120,7 @@ def test_include_test_overridden_by_default_excludes(self, mocker):
"codemodder.code_directory.Path.is_file",
return_value=True,
)
files = match_files(Path("."), include_paths=["**/tests/**"])
files = match_files(Path("."), include_paths=["tests/**"])
self._assert_expected(files, [])

def test_include_test_without_default_includes(self, mocker):
Expand Down
Loading