diff --git a/scripts/pr_review.py b/scripts/pr_review.py index e8dd3fb3..e0256edd 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -4534,7 +4534,7 @@ def marker_blocks(body: str, marker: re.Pattern[str], strip_blockquote: bool = F """Return the review body's sections whose own heading matches `marker`. Each is sliced from its heading to the next sibling heading or the end of its own region. - Past a region's first section, only the sibling heading it ends at can open another. + Past a region's first section, only a sibling heading some section ends at can open another. Shared by `suppressed_blocks`, `previously_missed_blocks`, and `outside_diff_blocks`. `strip_blockquote` affects heading detection only, never the returned content. A summary split @@ -4565,9 +4565,10 @@ def marker_blocks(body: str, marker: re.Pattern[str], strip_blockquote: bool = F bare = COUNT.search(line) and not OPENING_COUNT.match(line) if marker.search(line) and (HEADING.match(line) or bare) and not entry_heading(line): start = i - while start < len(scan_lines) and marker.search(scan_lines[start]): + while start < len(scan_lines): end = sibling_heading(scan_lines, start) - blocks.append("\n".join(raw_lines[start:end])) + if marker.search(scan_lines[start]): + blocks.append("\n".join(raw_lines[start:end])) start = end break return blocks diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index 1382e7e6..1f3a4206 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -1603,6 +1603,48 @@ def test_a_section_ends_where_its_sibling_s_heading_starts(self) -> None: self.assertEqual(1, out.count("The suppressed finding.")) self.assertEqual(1, out.count("The previously missed finding.")) + def test_sections_alternating_in_one_wrapper_are_each_read(self) -> None: + """The chain of bounds runs through a section of the other kind rather than stopping there. + + Stopped at the first sibling of the other kind, the second suppressed section reached no + block. The digest counted one suppressed finding of three, under `shapes=ok`. + """ + shared = ( + f"{OVERVIEW}\n
\nReview details\n\n" + "Suppressed comments (1)\n\nThe first suppressed finding.\n\n" + "Previously missed (1)\n\nThe previously missed finding.\n\n" + "Suppressed comments (2)\n\nThe second suppressed finding.\n\n" + "The third suppressed finding.\n
\n" + ) + suppressed = pr_review.suppressed_blocks(shared) + self.assertEqual(2, len(suppressed)) + self.assertIn("The first suppressed finding.", suppressed[0]) + self.assertIn("The second suppressed finding.", suppressed[1]) + self.assertIn("The third suppressed finding.", suppressed[1]) + self.assertNotIn("The previously missed finding.", "".join(suppressed)) + missed = pr_review.previously_missed_blocks(shared) + self.assertEqual(1, len(missed)) + self.assertNotIn("suppressed finding.", missed[0]) + self.answer(payload([review(body=shared)])) + out, _ = pr_review.digest("o", "r", 7) + self.assertIn("suppressed=3", out) + self.assertIn("shapes=ok", out) + for finding in ( + "first suppressed", + "previously missed", + "second suppressed", + "third suppressed", + ): + self.assertEqual(1, out.count(f"The {finding} finding.")) + swapped = ( + f"{OVERVIEW}\n
\nReview details\n\n" + "Previously missed (1)\n\nThe first.\n\n" + "Suppressed comments (1)\n\nThe second.\n\n" + "Previously missed (1)\n\nThe third.\n
\n" + ) + self.assertEqual(2, len(pr_review.previously_missed_blocks(swapped))) + self.assertEqual([], pr_review.unrecognized_in(swapped)) + def test_a_finding_entry_is_no_section_whatever_its_title_says(self) -> None: """An entry's title is free text, and either section's marker words can appear in it.