Skip to content

Commit c8a2398

Browse files
committed
fix: check a commit with an empty message as empty, not as HEAD
git log listing dropped such a commit, and piped empty text reads to the CLI as no input, so it checked HEAD (the merge commit) instead. The commit is now kept and its empty message goes to the CLI as an empty file, where allow_empty_commits judges it.
1 parent 73fd34f commit c8a2398

2 files changed

Lines changed: 65 additions & 11 deletions

File tree

‎main.py‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import re
1818
import subprocess
1919
import sys
20+
import tempfile
2021
import uuid
2122
from dataclasses import dataclass, field
2223
from typing import Any
@@ -567,14 +568,13 @@ def parse_commit_messages(output: str) -> list[Commit]:
567568
568569
The fields alternate hash, message, hash, message; git puts a newline
569570
between commits, which lands on the front of the next hash and is
570-
stripped along with the message's trailing newline. Commits are paired
571-
before empty messages are dropped, so a blank message never shifts the
572-
hashes of the commits after it.
571+
stripped along with the message's trailing newline. A commit with an
572+
empty message is kept, as ``(sha, "")``: it is a commit of the pull
573+
request like any other, and ``allow_empty_commits`` is the rule that
574+
judges it.
573575
"""
574576
fields = [f.strip("\n") for f in output.split(COMMIT_MESSAGE_DELIMITER)]
575-
return [
576-
(sha, message) for sha, message in zip(fields[0::2], fields[1::2]) if message
577-
]
577+
return [(sha, message) for sha, message in zip(fields[0::2], fields[1::2]) if sha]
578578

579579

580580
def _messages_in_range(revision_range: str) -> list[Commit]:
@@ -831,12 +831,31 @@ def run_pr_message_checks(pr_commits: list[Commit]) -> list[ScopeResult]:
831831
if author:
832832
names = ("GIT_AUTHOR_NAME", "GIT_AUTHOR_EMAIL")
833833
env = {key: value for key, value in zip(names, author) if value} or None
834-
results.append(
835-
check_scope(label, ["--message"], input_text=msg, sha=sha, env=env)
836-
)
834+
if msg.strip():
835+
scope = check_scope(label, ["--message"], input_text=msg, sha=sha, env=env)
836+
else:
837+
scope = check_empty_message(label, sha, env)
838+
results.append(scope)
837839
return results
838840

839841

842+
def check_empty_message(
843+
label: str, sha: str, env: dict[str, str] | None
844+
) -> ScopeResult:
845+
"""Check a commit whose message is empty, from an empty file.
846+
847+
Piped, an empty message reads to the CLI as no input at all, and it
848+
checks HEAD in its place: the merge commit, which passes. A file names
849+
the message, so the empty one is judged (``allow_empty_commits``).
850+
"""
851+
fd, path = tempfile.mkstemp(prefix="commit-msg-", dir=os.getenv("RUNNER_TEMP"))
852+
os.close(fd)
853+
try:
854+
return check_scope(label, ["--message", path], sha=sha, env=env)
855+
finally:
856+
os.unlink(path)
857+
858+
840859
def run_other_checks(args: list[str], rev: str | None = None) -> list[ScopeResult]:
841860
"""Run each non-message check (branch, author) once, as its own scope.
842861

‎main_test.py‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -245,9 +245,12 @@ def test_the_format_asks_git_for_the_hash_and_the_body(self):
245245
self.assertEqual(main.COMMIT_LOG_FORMAT, "--pretty=format:%H%x00%B%x00")
246246
self.assertNotIn("\x00", main.COMMIT_LOG_FORMAT)
247247

248-
def test_an_empty_message_does_not_shift_the_hashes_after_it(self):
248+
def test_an_empty_message_is_kept_and_does_not_shift_the_hashes(self):
249249
output = f"{SHA_A}\x00\n\x00\n{SHA_B}\x00feat: second\n\x00"
250-
self.assertEqual(main.parse_commit_messages(output), [(SHA_B, "feat: second")])
250+
self.assertEqual(
251+
main.parse_commit_messages(output),
252+
[(SHA_A, ""), (SHA_B, "feat: second")],
253+
)
251254

252255
def test_empty_output_is_no_commits(self):
253256
self.assertEqual(main.parse_commit_messages(""), [])
@@ -4186,3 +4189,35 @@ def test_comment_client_uses_github_api_url(self):
41864189
main.add_pr_comments([fail_scope()])
41874190
_, kwargs = github_module.Github.call_args
41884191
self.assertEqual(kwargs["base_url"], "https://ghe.example.com/api/v3")
4192+
4193+
4194+
class TestEmptyCommitMessage(unittest.TestCase):
4195+
"""An empty message is checked as empty, never as HEAD's message."""
4196+
4197+
def test_an_empty_message_goes_to_the_cli_as_an_empty_file(self):
4198+
seen: list[tuple[list[str], str | None, str]] = []
4199+
4200+
def run(command, **kwargs):
4201+
path = command[-1]
4202+
with open(path, encoding="utf-8") as f:
4203+
seen.append((command, kwargs.get("input"), f.read()))
4204+
return MagicMock(
4205+
returncode=1,
4206+
stdout=json_output(make_check("allow_empty_commits", status="fail")),
4207+
stderr="",
4208+
)
4209+
4210+
with (
4211+
patch("main.subprocess.run", side_effect=run),
4212+
patch("main._local_author", return_value=None),
4213+
):
4214+
results = main.run_pr_message_checks([("a1", "")])
4215+
command, piped, content = seen[0]
4216+
self.assertEqual(command[:4], ["commit-check", "--format", "json", "--message"])
4217+
self.assertIsNone(piped)
4218+
self.assertEqual(content, "")
4219+
self.assertFalse(os.path.exists(command[-1]))
4220+
self.assertEqual(
4221+
[(s.label, s.sha, s.status) for s in results],
4222+
[("Commit 1/1", "a1", "fail")],
4223+
)

0 commit comments

Comments
 (0)