diff mbox series

[06/11] patchtest: test_patch: improve patch-specific tests

Message ID 20260514194207.1958325-7-tgamblin@baylibre.com
State New
Headers show
Series patchtest: improve testing coverage | expand

Commit Message

Trevor Gamblin May 14, 2026, 7:42 p.m. UTC
[YOCTO #16247]

- Create two new tests:
  1. test_patch_malformed_context, for catching empty lines which might be
     treated as context lines (and ensuring they begin with proper diff
     characters);
  2. test_patch_trailing_whitespace, for catching trailing spaces/tabs on
     new lines.

- Be explicit about line numbers when a patch has one or more malformed
  lines. To do this correctly, we actually have to rework the test a bit
  and extract the inner patch hunks for checking, otherwise we are
  functionally testing the entire diff and reporting malformed lines for
  the whole commit. This means, for example, that we could see something
  like this for an mbox adding a patch with only 29 lines:

  |FAIL: test patch malformed context: Malformed patch line: context line is missing its leading space prefix (test_patch.TestPatch.test_patch_malformed_context)
  |    Patch: 174c24d6e87aeae631bc0a7bb1ba983cf8def4de.patch
  |    Lines: 23, 36

- When no new patches are introduced, patchtest reports that there are
  no *CVE* patches introduced, but this is not the only type we
  potentially carry. Remove that acronym from the message so the warning
  doesn't mislead users.

- When running test_signed_off_by_presence() and
  test_upstream_status_presence_format(), don't test the existing tags
  if the patch has only been modified to carry forward. This way we
  avoid sending false positive emails out during some upgrades and patch
  refreshes.

AI-Generated: Uses Claude Code

Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
---
 meta/lib/patchtest/tests/test_patch.py | 80 +++++++++++++++++++++++++-
 1 file changed, 78 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/meta/lib/patchtest/tests/test_patch.py b/meta/lib/patchtest/tests/test_patch.py
index 9f1d7d2c8f..42a4f7f9ac 100644
--- a/meta/lib/patchtest/tests/test_patch.py
+++ b/meta/lib/patchtest/tests/test_patch.py
@@ -10,6 +10,23 @@  import os
 import patchtest_patterns
 import pyparsing
 
+
+def _format_line_numbers(nums):
+    """Format a sorted list of line numbers as compact ranges, e.g. [1,2,3,7] -> '1-3, 7'."""
+    if not nums:
+        return ''
+    nums = sorted(set(nums))
+    parts = []
+    start = end = nums[0]
+    for n in nums[1:]:
+        if n == end + 1:
+            end = n
+        else:
+            parts.append(str(start) if start == end else '%d-%d' % (start, end))
+            start = end = n
+    parts.append(str(start) if start == end else '%d-%d' % (start, end))
+    return ', '.join(parts)
+
 class TestPatch(base.Base):
 
     @classmethod
@@ -17,7 +34,7 @@  class TestPatch(base.Base):
         cls.newpatches = []
         # get just those relevant patches: new software patches
         for patch in cls.patchset:
-            if patch.path.endswith('.patch') and patch.is_added_file:
+            if patch.path.endswith('.patch') and (patch.is_added_file or patch.is_modified_file):
                 cls.newpatches.append(patch)
 
         cls.mark = str(patchtest_patterns.signed_off_by_prefix).strip('"')
@@ -29,7 +46,7 @@  class TestPatch(base.Base):
         if self.unidiff_parse_error:
             self.skip('Parse error %s' % self.unidiff_parse_error)
         if not TestPatch.newpatches:
-            self.skip('No new CVE patches introduced')
+            self.skip('No new patches introduced')
 
     def test_upstream_status_presence_format(self):
         if not TestPatch.newpatches:
@@ -39,6 +56,8 @@  class TestPatch(base.Base):
         standard_format = "Upstream-Status: <Valid status>"
 
         for newpatch in TestPatch.newpatches:
+            if not newpatch.is_added_file:
+                continue
             payload = newpatch.__str__()
             lines = payload.splitlines()
 
@@ -147,6 +166,8 @@  class TestPatch(base.Base):
             self.skip("There are no new software patches, no reason to test %s presence" % TestPatch.mark)
 
         for newpatch in TestPatch.newpatches:
+            if not newpatch.is_added_file:
+                continue
             payload = newpatch.__str__()
             for line in payload.splitlines():
                 if patchtest_patterns.patchmetadata_regex.match(line):
@@ -169,3 +190,58 @@  class TestPatch(base.Base):
                 if not tag_found:
                     self.fail('Missing or incorrectly formatted CVE tag in patch file. Correct or include the CVE tag in the patch with format: "CVE: CVE-YYYY-XXXX"',
                               commit=commit)
+
+    def _iter_inner_patch_hunk_lines(self, newpatch):
+        """Yield (lineno, raw) for each newly introduced hunk content line in a .patch file.
+
+        Reads from the outer diff's added lines so we check exactly what the
+        submission introduces, regardless of what may already exist on disk.
+        lineno is 1-based among the added lines of the inner patch content.
+        """
+        # Collect inner patch lines from the outer diff's '+' lines.
+        # '++++ b/file' (outer '+' + inner '+++ b/file') starts with '+++' and is
+        # excluded, but in_hunk will already be False from the preceding '--- ' line.
+        inner_lines = []
+        for line in str(newpatch).splitlines(keepends=True):
+            if line.startswith('+') and not line.startswith('+++ '):
+                inner_lines.append(line[1:])
+
+        in_hunk = False
+        for lineno, raw in enumerate(inner_lines, start=1):
+            if raw.startswith('diff --git') or raw.startswith('--- ') or raw.startswith('+++ ') or raw.startswith('index '):
+                in_hunk = False
+            elif raw.rstrip('\n\r') in ('-- ', '--'):
+                # git format-patch email signature separator; nothing after this is hunk content
+                in_hunk = False
+            elif raw.startswith('@@ '):
+                in_hunk = True
+            elif in_hunk:
+                yield lineno, raw
+
+    def test_patch_malformed_context(self):
+        for newpatch in TestPatch.newpatches:
+            bad_linenos = []
+            for lineno, raw in self._iter_inner_patch_hunk_lines(newpatch):
+                # git strips trailing whitespace from blank context lines, leaving bare '\n'
+                if raw.rstrip('\n\r') and not raw.startswith((' ', '+', '-', '\\')):
+                    bad_linenos.append(lineno)
+            if bad_linenos:
+                self.fail(
+                    'Malformed patch line: context line is missing its leading space prefix',
+                    data=[
+                        ('Patch', os.path.basename(newpatch.path)),
+                        ('Lines', _format_line_numbers(bad_linenos)),
+                    ],
+                )
+
+    def test_patch_trailing_whitespace(self):
+        for newpatch in TestPatch.newpatches:
+            for lineno, raw in self._iter_inner_patch_hunk_lines(newpatch):
+                if raw.startswith('+') and raw.rstrip('\n\r') != raw.rstrip():
+                    self.fail(
+                        'Trailing whitespace in added line',
+                        data=[
+                            ('Patch', os.path.basename(newpatch.path)),
+                            ('Line', repr(raw)),
+                        ],
+                    )