Skip to content

Commit 6b273bd

Browse files
authored
feat(eip_checklist): Print warnings + embed links to test functions (#2103)
* feat(checklist): Add checklist warnings * feat(checklist): add relative links to the tests
1 parent bce61c1 commit 6b273bd

File tree

2 files changed

+97
-4
lines changed

2 files changed

+97
-4
lines changed

docs/writing_tests/checklist_templates/eip_testing_checklist_template.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ Depending on the changes introduced by an EIP, the following template is the min
99
| --------------------- | ----------------------- | ---------- |
1010
| TOTAL_CHECKLIST_ITEMS | COVERED_CHECKLIST_ITEMS | PERCENTAGE |
1111

12+
<!-- WARNINGS LINE -->
13+
1214
## General
1315

1416
#### Code coverage

src/pytest_plugins/filler/eip_checklist.py

Lines changed: 95 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
import re
1111
from dataclasses import dataclass, field
1212
from pathlib import Path
13-
from typing import Dict, List, Set, Tuple
13+
from typing import ClassVar, Dict, List, Set, Tuple, Type
1414

1515
import pytest
1616

@@ -59,6 +59,7 @@ def pytest_addoption(parser: pytest.Parser):
5959
TEMPLATE_CONTENT = TEMPLATE_PATH.read_text()
6060
EXTERNAL_COVERAGE_FILE_NAME = "eip_checklist_external_coverage.txt"
6161
NOT_APPLICABLE_FILE_NAME = "eip_checklist_not_applicable.txt"
62+
WARNINGS_LINE = "<!-- WARNINGS LINE -->"
6263

6364

6465
@pytest.hookimpl(tryfirst=True)
@@ -113,8 +114,11 @@ def __str__(self) -> str:
113114
status = "✅"
114115
tests = self.external_coverage_reason
115116
elif self.covered:
116-
status = "✅"
117-
tests = ", ".join(sorted(self.tests))
117+
if self.not_applicable:
118+
status = "❓"
119+
else:
120+
status = "✅"
121+
tests = ", ".join(sorted(map(resolve_test_link, self.tests)))
118122
elif self.not_applicable:
119123
status = "N/A"
120124
tests = self.not_applicable_reason
@@ -156,6 +160,75 @@ def resolve_id(item_id: str) -> Set[str]:
156160
return covered_ids
157161

158162

163+
def resolve_test_link(test_id: str) -> str:
164+
"""Resolve a test ID to a test link."""
165+
# test_id example: tests/fork/eip1234_some_eip/test_file.py::test_function[test_param1-...]
166+
# Relative path: ../../../../tests/fork/eip1234_some_eip/test_file/test_function/
167+
pattern = r"(.*)\.py::(\w+)"
168+
match = re.match(pattern, test_id)
169+
if not match:
170+
return test_id
171+
return f"[{test_id}](../../../../{match.group(1)}/{match.group(2)}/)"
172+
173+
174+
ALL_CHECKLIST_WARNINGS: Dict[str, Type["ChecklistWarning"]] = {}
175+
176+
177+
@dataclass(kw_only=True)
178+
class ChecklistWarning:
179+
"""Represents an EIP checklist warning."""
180+
181+
title: ClassVar[str] = ""
182+
details: List[str]
183+
184+
def __init_subclass__(cls) -> None:
185+
"""Register the checklist warning subclass."""
186+
super().__init_subclass__()
187+
assert cls.title, "Title must be set"
188+
if cls.title in ALL_CHECKLIST_WARNINGS:
189+
raise ValueError(f"Duplicate checklist warning class: {cls}")
190+
ALL_CHECKLIST_WARNINGS[cls.title] = cls
191+
192+
def lines(self) -> List[str]:
193+
"""Return the lines of the checklist warning."""
194+
return ["", f"### {self.title}", ""] + self.details + [""]
195+
196+
@classmethod
197+
def from_items(cls, all_items: Dict[str, EIPItem]) -> "ChecklistWarning | None":
198+
"""Generate a checklist warning from a list of items."""
199+
raise NotImplementedError(f"from_items not implemented for {cls}")
200+
201+
202+
class ConflictingChecklistItemsWarning(ChecklistWarning):
203+
"""Represents a conflicting checklist items warning."""
204+
205+
title: ClassVar[str] = "Conflicting Checklist Items"
206+
207+
@classmethod
208+
def from_items(cls, all_items: Dict[str, EIPItem]) -> ChecklistWarning | None:
209+
"""Generate a conflicting checklist items warning from a list of items."""
210+
conflicting_items = [
211+
item for item in all_items.values() if item.not_applicable and item.covered
212+
]
213+
if not conflicting_items:
214+
return None
215+
216+
details = [
217+
"The following checklist items were marked both as not applicable and covered:",
218+
"",
219+
"| ID | Description | Not Applicable | Tests |",
220+
"|---|---|---|---|",
221+
]
222+
for item in conflicting_items:
223+
details.append(
224+
f"| {item.id} | {item.description} | "
225+
+ f"{item.not_applicable_reason} | "
226+
+ f"{', '.join(sorted(map(resolve_test_link, item.tests)))} |"
227+
)
228+
229+
return cls(details=details)
230+
231+
159232
@dataclass(kw_only=True)
160233
class EIP:
161234
"""Represents an EIP and its checklist."""
@@ -171,7 +244,7 @@ def add_covered_test(self, checklist_id: str, node_id: str) -> None:
171244
@property
172245
def covered_items(self) -> int:
173246
"""Return the number of covered items."""
174-
return sum(1 for item in self.items.values() if item.covered)
247+
return sum(1 for item in self.items.values() if item.covered and not item.not_applicable)
175248

176249
@property
177250
def total_items(self) -> int:
@@ -188,6 +261,15 @@ def completeness_emoji(self) -> str:
188261
"""Return the completeness emoji."""
189262
return "🟢" if self.percentage == 100 else "🟡" if self.percentage > 50 else "🔴"
190263

264+
@property
265+
def warnings(self) -> List[ChecklistWarning]:
266+
"""Return the detected inconsistencies in the checklist."""
267+
warnings = []
268+
for warning_cls in ALL_CHECKLIST_WARNINGS.values():
269+
if warning := warning_cls.from_items(self.items):
270+
warnings.append(warning)
271+
return warnings
272+
191273
def mark_not_applicable(self):
192274
"""Read the not-applicable items from the EIP."""
193275
if self.path is None:
@@ -264,6 +346,15 @@ def generate_filled_checklist_lines(self) -> List[str]:
264346
# Replace the title line with the EIP number
265347
lines[lines.index(TITLE_LINE)] = f"# EIP-{self.number} Test Checklist"
266348

349+
# Last, add the warnings if there are any, this must be the last thing we do
350+
# to avoid shifting the lines below the percentage line
351+
if self.warnings:
352+
warnings_line_idx = lines.index(WARNINGS_LINE)
353+
warnings_lines = ["", "## ⚠️ Checklist Warnings ⚠️", ""]
354+
for warning in self.warnings:
355+
warnings_lines.extend(warning.lines())
356+
lines[warnings_line_idx:warnings_line_idx] = warnings_lines
357+
267358
return lines
268359

269360
def generate_filled_checklist(self, output_dir: Path) -> Path:

0 commit comments

Comments
 (0)