-
Notifications
You must be signed in to change notification settings - Fork 44
Calculate PR-style diff #532
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
Changes from all commits
54e726a
7a4e87f
2e8dc23
c0575ea
7a1270c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import os | ||
| import pathlib | ||
| import subprocess | ||
| import uuid | ||
|
|
||
| import pytest | ||
|
|
||
|
|
||
| @pytest.fixture | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This and below seems to be copied from test_main, right ? Could you deduplicate it ? It's ok to move it here, but delete the corresponding copied code from test_main
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call. Removed from |
||
| def in_integration_env(integration_env, integration_dir): | ||
| curdir = os.getcwd() | ||
| os.chdir(integration_dir) | ||
| yield integration_dir | ||
| os.chdir(curdir) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def integration_dir(tmp_path: pathlib.Path): | ||
| test_dir = tmp_path / "integration_test" | ||
| test_dir.mkdir() | ||
| return test_dir | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def file_path(integration_dir): | ||
| return integration_dir / "foo.py" | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def write_file(file_path): | ||
| def _(*variables): | ||
| content = "import os" | ||
| for i, var in enumerate(variables): | ||
| content += f"""\nif os.environ.get("{var}"):\n {i}\n""" | ||
| file_path.write_text(content, encoding="utf8") | ||
|
|
||
| return _ | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def run_coverage(file_path, integration_dir): | ||
| def _(*variables): | ||
| subprocess.check_call( | ||
| ["coverage", "run", "--parallel", file_path.name], | ||
| cwd=integration_dir, | ||
| env=os.environ | dict.fromkeys(variables, "1"), | ||
| ) | ||
|
|
||
| return _ | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def commit(integration_dir): | ||
| def _(): | ||
| subprocess.check_call( | ||
| ["git", "add", "."], | ||
| cwd=integration_dir, | ||
| ) | ||
| subprocess.check_call( | ||
| ["git", "commit", "-m", str(uuid.uuid4())], | ||
| cwd=integration_dir, | ||
| env={ | ||
| "GIT_AUTHOR_NAME": "foo", | ||
| "GIT_AUTHOR_EMAIL": "foo", | ||
| "GIT_COMMITTER_NAME": "foo", | ||
| "GIT_COMMITTER_EMAIL": "foo", | ||
| "GIT_CONFIG_GLOBAL": "/dev/null", | ||
| "GIT_CONFIG_SYSTEM": "/dev/null", | ||
| }, | ||
| ) | ||
|
|
||
| return _ | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def integration_env(integration_dir, write_file, run_coverage, commit, request): | ||
| subprocess.check_call(["git", "init", "-b", "main"], cwd=integration_dir) | ||
| # diff coverage reads the "origin/{...}" branch so we simulate an origin remote | ||
| subprocess.check_call(["git", "remote", "add", "origin", "."], cwd=integration_dir) | ||
| write_file("A", "B") | ||
| commit() | ||
|
|
||
| add_branch_mark = request.node.get_closest_marker("add_branches") | ||
| for additional_branch in add_branch_mark.args if add_branch_mark else []: | ||
| subprocess.check_call( | ||
| ["git", "switch", "-c", additional_branch], | ||
| cwd=integration_dir, | ||
| ) | ||
|
|
||
| subprocess.check_call( | ||
| ["git", "switch", "-c", "branch"], | ||
| cwd=integration_dir, | ||
| ) | ||
|
|
||
| write_file("A", "B", "C", "D") | ||
| commit() | ||
|
|
||
| run_coverage("A", "C") | ||
| subprocess.check_call(["git", "fetch", "origin"], cwd=integration_dir) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import subprocess | ||
|
|
||
| from coverage_comment import coverage | ||
| from coverage_comment import subprocess as subprocess_module | ||
|
|
||
|
|
||
| def test_get_added_lines( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't believe this is strictly necessary as it's more testing git than
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm good keeping it :) |
||
| commit, file_path, integration_dir, in_integration_env, write_file | ||
| ): | ||
| """ | ||
| Lines added in the base_ref should not appear as added in HEAD | ||
| """ | ||
| git = subprocess_module.Git() | ||
| relative_file_path = file_path.relative_to(integration_dir) | ||
|
|
||
| assert coverage.get_added_lines(git, "main") == { | ||
| relative_file_path: list(range(7, 13)) # Line numbers start at 1 | ||
| } | ||
|
|
||
| subprocess.check_call(["git", "switch", "main"], cwd=integration_dir) | ||
| write_file("E", "F") | ||
| commit() | ||
| subprocess.check_call(["git", "push", "origin", "main"], cwd=integration_dir) | ||
| subprocess.check_call(["git", "switch", "branch"], cwd=integration_dir) | ||
|
|
||
| assert coverage.get_added_lines(git, "main") == { | ||
| relative_file_path: list(range(7, 13)) # Line numbers start at 1 | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll look into the e2e tests to test this since I'm not entirely sure how the actions/checkout action handles this. I wonder if you can get away with not setting
fetch_depthhere since agit fetchis called later.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oooooh, that's the comment I missed that caused the issue. I'm sorry @stickperson I completely missed that 😅