Skip to content

Commit

Permalink
fix: better detect merge-commits (#397)
Browse files Browse the repository at this point in the history
Apparently a common thing for CIs to do is create a merge-commit for changes
in a branch before running tests and stuff.
This means that - especially for not-directly-supported CIs - we would maybe
return a SHA for a commit that didn't exist in the branch.

These changes fix that by checking to see if the current commit is a merge commit.
If it is we return the second parent, the most recent commit before the current one.

Q: What if the current latest commit in the branch is a merge commit?
Well in this case the parent, which is also part of the branch, will have the coverage.

Users can still provide a commit sha value to override this behavior.

closes #372
  • Loading branch information
giovanni-guidini authored Mar 6, 2024
1 parent 92d7e89 commit ff1c300
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 9 deletions.
18 changes: 16 additions & 2 deletions codecov_cli/helpers/versioning_systems.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,20 @@ def is_available(cls):

def get_fallback_value(self, fallback_field: FallbackFieldEnum):
if fallback_field == FallbackFieldEnum.commit_sha:
# here we will get the commit SHA of the latest commit
# that is NOT a merge commit
p = subprocess.run(
# List current commit parent's SHA
["git", "rev-parse", "HEAD^@"],
capture_output=True,
)
parents_hash = p.stdout.decode().strip().splitlines()
if len(parents_hash) == 2:
# IFF the current commit is a merge commit it will have 2 parents
# We return the 2nd one - The commit that came from the branch merged into ours
return parents_hash[1]
# At this point we know the current commit is not a merge commit
# so we get it's SHA and return that
p = subprocess.run(["git", "log", "-1", "--format=%H"], capture_output=True)
if p.stdout:
return p.stdout.decode().strip()
Expand All @@ -56,7 +70,7 @@ def get_fallback_value(self, fallback_field: FallbackFieldEnum):
return branch_name if branch_name != "HEAD" else None

if fallback_field == FallbackFieldEnum.slug:
# if there are multiple remotes, we will prioritize using the one called 'origin' if it exsits, else we will use the first one in 'git remote' list
# if there are multiple remotes, we will prioritize using the one called 'origin' if it exists, else we will use the first one in 'git remote' list

p = subprocess.run(["git", "remote"], capture_output=True)

Expand All @@ -78,7 +92,7 @@ def get_fallback_value(self, fallback_field: FallbackFieldEnum):
return parse_slug(remote_url)

if fallback_field == FallbackFieldEnum.git_service:
# if there are multiple remotes, we will prioritize using the one called 'origin' if it exsits, else we will use the first one in 'git remote' list
# if there are multiple remotes, we will prioritize using the one called 'origin' if it exists, else we will use the first one in 'git remote' list

p = subprocess.run(["git", "remote"], capture_output=True)
if not p.stdout:
Expand Down
23 changes: 17 additions & 6 deletions tests/helpers/test_versioning_systems.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,28 @@

class TestGitVersioningSystem(object):
@pytest.mark.parametrize(
"commit_sha,expected", [("", None), (b" random_sha ", "random_sha")]
"runs_output,expected",
[
# No output for parents nor commit
([b"", b""], None),
# No output for parents, commit has SHA
([b"", b" random_sha"], "random_sha"),
# Commit is NOT a merge-commit
([b" parent_sha", b" random_sha "], "random_sha"),
# Commit IS a merge-commit
([b" parent_sha0\nparent_sha1", b" random_sha"], "parent_sha1"),
],
)
def test_commit_sha(self, mocker, commit_sha, expected):
mocked_subprocess = MagicMock()
def test_commit_sha(self, mocker, runs_output, expected):
mocked_subprocess = [
MagicMock(**{"stdout": runs_output[0]}),
MagicMock(**{"stdout": runs_output[1]}),
]
mocker.patch(
"codecov_cli.helpers.versioning_systems.subprocess.run",
return_value=mocked_subprocess,
side_effect=mocked_subprocess,
)

mocked_subprocess.stdout = commit_sha

assert (
GitVersioningSystem().get_fallback_value(FallbackFieldEnum.commit_sha)
== expected
Expand Down
4 changes: 3 additions & 1 deletion tests/services/upload/test_coverage_file_finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,9 @@ def test_find_coverage_files_with_user_specified_files_not_found(self):
expected_paths = sorted([file.get_filename() for file in expected])
self.assertEqual(result, expected_paths)

def test_find_coverage_files_with_user_specified_files_in_default_ignored_folder(self):
def test_find_coverage_files_with_user_specified_files_in_default_ignored_folder(
self,
):
# Create some sample coverage files
coverage_files = [
self.project_root / "coverage.xml",
Expand Down

0 comments on commit ff1c300

Please sign in to comment.