From: Simon Glass <simon.glass@canonical.com>
The regex for extracting commit hashes from MR descriptions was matching
short numbers like "1" from the conversation log (e.g., "- 1 board built").
Fix by requiring commit hashes to be at least 7 characters, which is the
minimum length for git short hashes.
Shorten the argument name to 'desc' while we are here.
Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
---
tools/pickman/control.py | 10 +++++-----
tools/pickman/ftest.py | 18 ++++++++++++++++++
2 files changed, 23 insertions(+), 5 deletions(-)
@@ -651,23 +651,23 @@ def do_review(args, dbs): # pylint: disable=unused-argument
return 0
-def parse_mr_description(description):
+def parse_mr_description(desc):
"""Parse a pickman MR description to extract source and last commit
Args:
- description (str): MR description text
+ desc (str): MR description text
Returns:
tuple: (source_branch, last_commit_hash) or (None, None) if not parseable
"""
# Extract source branch from "## date: source_branch" line
- source_match = re.search(r'^## [^:]+: (.+)$', description, re.MULTILINE)
+ source_match = re.search(r'^## [^:]+: (.+)$', desc, re.MULTILINE)
if not source_match:
return None, None
source = source_match.group(1)
- # Extract commits from "- hash subject" lines
- commit_matches = re.findall(r'^- ([a-f0-9]+) ', description, re.MULTILINE)
+ # Extract commits from '- hash subject' lines (must be at least 7 chars)
+ commit_matches = re.findall(r'^- ([a-f0-9]{7,}) ', desc, re.MULTILINE)
if not commit_matches:
return None, None
@@ -1355,6 +1355,24 @@ Branch: cherry-abc"""
self.assertIsNone(source)
self.assertIsNone(last_hash)
+ def test_parse_mr_description_ignores_short_hashes(self):
+ """Test that short numbers in conversation log are not matched."""
+ description = """## 2025-01-15: us/next
+
+Branch: cherry-abc123
+
+Commits:
+- abc123a First commit
+- def456b Second commit
+
+### Conversation log
+- 1 board built (sandbox)
+- 2 tests passed"""
+ source, last_hash = control.parse_mr_description(description)
+ self.assertEqual(source, 'us/next')
+ # Should match def456b, not "1" or "2" from conversation log
+ self.assertEqual(last_hash, 'def456b')
+
class TestStep(unittest.TestCase):
"""Tests for step command."""