gavindiaz/awesome-quant
1
0
Fork 0
mirror of https://github.com/wilsonfreitas/awesome-quant.git synced 2026-11-02 04:16:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

Enhance README entry validation to allow up to five entries and update tests for multi-entry scenarios

Browse Source
This commit is contained in:
Wilson Freitas
2026-08-29 16:55:34 -03:00
parent 36e245ed57
commit d1f09ae0b3
2 changed files with 206 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/review_pr.py
+112 -61
View File
@@ -41,6 +41,7 @@ NO_TAG_SECTIONS = {
}
RECENT_CLOSED_PULL_DAYS = 365
MAX_README_ENTRIES_PER_PR = 5
CHECK_ORDER = (
"description",
"files",
@@ -198,7 +199,7 @@ def entries_represent_same_project(old_line: str, new_line: str) -> bool:
def analyze_readme_change(
base_readme: str,
head_readme: str,
) -> tuple[str | None, str | None, list[Finding]]:
) -> tuple[list[str], list[str], list[Finding]]:
added_lines, removed_lines = readme_changed_lines(base_readme, head_readme)
substantive_added = [line for line in added_lines if line.strip()]
substantive_removed = [line for line in removed_lines if line.strip()]
@@ -209,45 +210,54 @@ def analyze_readme_change(
line for line in substantive_removed if line.strip().startswith("- ")
]
findings: list[Finding] = []
if len(entry_lines) != 1:
if not 1 <= len(entry_lines) <= MAX_README_ENTRIES_PER_PR:
findings.append(
Finding(
"entry-count",
"expected exactly one added README entry line",
"expected between one and five added README entry lines",
)
)
return None, None, findings
return [], [], findings
unauthorized_additions = [
line for line in substantive_added if line != entry_lines[0]
line for line in substantive_added if line not in entry_lines
]
unauthorized_removals = [
line for line in substantive_removed if line not in removed_entry_lines
]
removed_entry_line = (
removed_entry_lines[0] if len(removed_entry_lines) == 1 else None
)
unmatched_entry_lines = list(entry_lines)
unmatched_removal = False
for removed_entry_line in removed_entry_lines:
matching_index = next(
(
index
for index, entry_line in enumerate(unmatched_entry_lines)
if entries_represent_same_project(removed_entry_line, entry_line)
),
None,
)
if matching_index is None:
unmatched_removal = True
break
unmatched_entry_lines.pop(matching_index)
invalid_update = bool(substantive_removed) and (
removed_entry_line is None
or bool(unauthorized_removals)
or not entries_represent_same_project(removed_entry_line, entry_lines[0])
bool(unauthorized_removals) or unmatched_removal
)
if unauthorized_additions or invalid_update:
findings.append(
Finding(
"content",
"README changes must add one entry or update the same project "
"README changes must add up to five entries or update the same projects "
"without other substantive edits",
)
)
return entry_lines[0], removed_entry_line, findings
return entry_lines, removed_entry_lines, findings
def remove_entry_line(readme_text: str, entry_line: str | None) -> str:
if entry_line is None:
return readme_text
def remove_entry_lines(readme_text: str, entry_lines: list[str]) -> str:
lines = readme_text.splitlines()
lines.remove(entry_line)
for entry_line in entry_lines:
lines.remove(entry_line)
return "\n".join(lines)
@@ -488,48 +498,26 @@ def repository_has_pull_request_duplicate(
return False
def review_pr(
repository_name: str,
pr_number: int,
def review_entry(
repository: Any,
client: Github,
pr_number: int,
line: str,
*,
now: datetime | None = None,
check_pull_request_duplicates: bool = True,
) -> tuple[list[Finding], str]:
current_time = now or datetime.now(timezone.utc)
repository = client.get_repo(repository_name)
pull_request = repository.get_pull(pr_number)
current_time: datetime,
head_readme: str,
duplicate_base_readme: str,
other_entry_lines: list[str],
check_pull_request_duplicates: bool,
readme_cache: dict[str, str],
) -> list[Finding]:
findings: list[Finding] = []
if not (pull_request.body or "").strip():
findings.append(Finding("description", "PR body is empty"))
files = list(pull_request.get_files())
if len(files) != 1 or files[0].filename != "README.md":
changed = ", ".join(file.filename for file in files) or "none"
findings.append(
Finding("files", f"only README.md may change, found: {changed}")
)
return findings, pull_request.title
base_readme = read_readme(repository, pull_request.base.sha)
head_readme = read_readme(repository, pull_request.head.sha)
entry_line, removed_entry_line, change_findings = analyze_readme_change(
base_readme,
head_readme,
)
findings.extend(change_findings)
if entry_line is None:
return findings, pull_request.title
line = entry_line
match = ENTRY_RE.match(line)
if not match:
findings.append(
Finding("format", "added README bullet does not match the entry regex")
)
return findings, pull_request.title
return findings
section = find_entry_section(head_readme, line)
name = match.group(1).strip()
@@ -635,21 +623,25 @@ def review_pr(
if not primary_github and not url_reachable(url):
findings.append(Finding("reachability", f"primary URL is not reachable: {url}"))
duplicate_base_readme = remove_entry_line(base_readme, removed_entry_line)
if readme_has_duplicate(duplicate_base_readme, name, [url, *github_urls]):
entry_urls = [url, *github_urls]
if readme_has_duplicate(duplicate_base_readme, name, entry_urls):
findings.append(
Finding("duplicates", "project name or URL already exists in README.md")
)
elif any(
entry_line_has_duplicate(other_line, name, entry_urls)
for other_line in other_entry_lines
):
findings.append(
Finding("duplicates", "project name or URL is duplicated within this PR")
)
elif check_pull_request_duplicates and repository_has_pull_request_duplicate(
repository,
pr_number,
name,
[url, *github_urls],
entry_urls,
now=current_time,
readme_cache={
pull_request.base.sha: base_readme,
pull_request.head.sha: head_readme,
},
readme_cache=readme_cache,
):
findings.append(
Finding(
@@ -658,7 +650,66 @@ def review_pr(
)
)
return findings, pull_request.title
return findings
def review_pr(
repository_name: str,
pr_number: int,
client: Github,
*,
now: datetime | None = None,
check_pull_request_duplicates: bool = True,
) -> tuple[list[Finding], str, int]:
current_time = now or datetime.now(timezone.utc)
repository = client.get_repo(repository_name)
pull_request = repository.get_pull(pr_number)
findings: list[Finding] = []
if not (pull_request.body or "").strip():
findings.append(Finding("description", "PR body is empty"))
files = list(pull_request.get_files())
if len(files) != 1 or files[0].filename != "README.md":
changed = ", ".join(file.filename for file in files) or "none"
findings.append(
Finding("files", f"only README.md may change, found: {changed}")
)
return findings, pull_request.title, 0
base_readme = read_readme(repository, pull_request.base.sha)
head_readme = read_readme(repository, pull_request.head.sha)
entry_lines, removed_entry_lines, change_findings = analyze_readme_change(
base_readme,
head_readme,
)
findings.extend(change_findings)
if not entry_lines:
return findings, pull_request.title, 0
duplicate_base_readme = remove_entry_lines(base_readme, removed_entry_lines)
readme_cache = {
pull_request.base.sha: base_readme,
pull_request.head.sha: head_readme,
}
for index, entry_line in enumerate(entry_lines):
findings.extend(
review_entry(
repository,
client,
pr_number,
entry_line,
current_time=current_time,
head_readme=head_readme,
duplicate_base_readme=duplicate_base_readme,
other_entry_lines=entry_lines[:index] + entry_lines[index + 1 :],
check_pull_request_duplicates=check_pull_request_duplicates,
readme_cache=readme_cache,
)
)
return findings, pull_request.title, len(entry_lines)
def main() -> int:
@@ -685,7 +736,7 @@ def main() -> int:
if not repository_name:
return fail("GITHUB_REPOSITORY is required")
client = Github(auth=Auth.Token(token))
findings, title = review_pr(
findings, title, entries_reviewed = review_pr(
repository_name,
pr_number,
client,
@@ -695,7 +746,7 @@ def main() -> int:
return fail(str(exc))
print(f"PR #{pr_number}: {title}")
print("Entries reviewed: 1")
print(f"Entries reviewed: {entries_reviewed}")
if args.skip_pull_request_duplicates:
print("Cross-PR duplicate check: skipped")
if not findings:
tests/test_review_pr.py
+94 -16
View File
@@ -25,6 +25,26 @@ VALID_PATCH = """@@ -1,1 +1,2 @@
+- [Fresh](https://github.com/example/fresh) - `Python` - Fresh project.
"""
TWO_ENTRY_PATCH = """@@ -1,1 +1,3 @@
## Trading & Backtesting
+- [Fresh One](https://github.com/example/fresh-one) - `Python` - Fresh project one.
+- [Fresh Two](https://github.com/example/fresh-two) - `Python` - Fresh project two.
"""
FIVE_ENTRY_PATCH = """@@ -1,1 +1,6 @@
## Trading & Backtesting
+- [Fresh One](https://github.com/example/fresh-one) - `Python` - Fresh project one.
+- [Fresh Two](https://github.com/example/fresh-two) - `Python` - Fresh project two.
+- [Fresh Three](https://github.com/example/fresh-three) - `Python` - Fresh project three.
+- [Fresh Four](https://github.com/example/fresh-four) - `Python` - Fresh project four.
+- [Fresh Five](https://github.com/example/fresh-five) - `Python` - Fresh project five.
"""
SIX_ENTRY_PATCH = FIVE_ENTRY_PATCH + (
"+- [Fresh Six](https://github.com/example/fresh-six) - "
"`Python` - Fresh project six.\n"
)
class FakePull:
def __init__(
@@ -176,7 +196,7 @@ class FakeClient:
self.requested_repositories.append(name)
if name == "owner/list":
return self.repository
if name == "example/fresh":
if name.startswith("example/"):
return self.project_repository
raise AssertionError(f"unexpected repository: {name}")
@@ -337,7 +357,7 @@ class PullRequestDuplicateTests(unittest.TestCase):
)
client = FakeClient(repository)
with patch("scripts.review_pr.url_reachable", return_value=True):
findings, title = review_pr(
findings, title, _entry_count = review_pr(
"owner/list",
10,
client,
@@ -489,7 +509,7 @@ class PullRequestDuplicateTests(unittest.TestCase):
client = FakeClient(repository)
with patch("scripts.review_pr.url_reachable", return_value=True):
findings, _title = review_pr(
findings, _title, _entry_count = review_pr(
"owner/list",
10,
client,
@@ -509,7 +529,7 @@ class PullRequestDuplicateTests(unittest.TestCase):
client = FakeClient(repository)
with patch("scripts.review_pr.url_reachable", return_value=True):
findings, _title = review_pr(
findings, _title, _entry_count = review_pr(
"owner/list",
10,
client,
@@ -581,7 +601,7 @@ class ValidationPipelineTests(unittest.TestCase):
"scripts.review_pr.url_reachable",
return_value=reachable,
):
findings, _title = review_pr(
findings, _title, _entry_count = review_pr(
"owner/list",
10,
client,
@@ -605,7 +625,7 @@ class ValidationPipelineTests(unittest.TestCase):
client = FakeClient(repository)
with patch("scripts.review_pr.url_reachable") as url_checker:
findings, _title = review_pr("owner/list", 10, client, now=NOW)
findings, _title, _entry_count = review_pr("owner/list", 10, client, now=NOW)
self.assertEqual(findings, [])
url_checker.assert_not_called()
@@ -625,7 +645,7 @@ class ValidationPipelineTests(unittest.TestCase):
client = FakeClient(repository)
with patch("scripts.review_pr.url_reachable", return_value=True) as url_checker:
findings, _title = review_pr("owner/list", 10, client, now=NOW)
findings, _title, _entry_count = review_pr("owner/list", 10, client, now=NOW)
self.assertEqual(findings, [])
url_checker.assert_called_once_with("https://example.com/fresh")
@@ -781,12 +801,35 @@ class ValidationPipelineTests(unittest.TestCase):
]
self.assertIn("files", self.review(files=files))
def test_rejects_multiple_entries(self):
patch_text = VALID_PATCH + (
"+- [Other](https://github.com/example/other) - "
"`Python` - Other project.\n"
def test_accepts_two_entries(self):
self.assertEqual(self.review(patch_text=TWO_ENTRY_PATCH), set())
def test_accepts_five_entries(self):
self.assertEqual(self.review(patch_text=FIVE_ENTRY_PATCH), set())
def test_rejects_more_than_five_entries(self):
self.assertIn("entry-count", self.review(patch_text=SIX_ENTRY_PATCH))
def test_validates_every_entry_in_a_multi_entry_pr(self):
patch_text = TWO_ENTRY_PATCH.replace(
"Fresh project two.",
"Fresh project two",
)
self.assertIn("entry-count", self.review(patch_text=patch_text))
findings = self.review(patch_text=patch_text)
self.assertIn("period", findings)
self.assertNotIn("entry-count", findings)
def test_rejects_duplicate_entries_within_a_multi_entry_pr(self):
patch_text = TWO_ENTRY_PATCH.replace(
"https://github.com/example/fresh-two",
"https://github.com/example/fresh-one",
)
findings = self.review(patch_text=patch_text)
self.assertIn("duplicates", findings)
def test_rejects_malformed_entry(self):
patch_text = """@@ -1,1 +1,2 @@
@@ -993,12 +1036,47 @@ class MainTests(unittest.TestCase):
return result, stdout.getvalue(), stderr.getvalue()
def test_success_reports_passed_checks(self):
result, stdout, _stderr = self.run_main(([], "Add Fresh"))
result, stdout, _stderr = self.run_main(([], "Add Fresh", 1))
self.assertEqual(result, 0)
self.assertIn("description: pass", stdout)
self.assertIn("duplicates: pass", stdout)
def test_success_reports_actual_entry_count(self):
changed_files = [
SimpleNamespace(filename="README.md", patch=FIVE_ENTRY_PATCH)
]
base_readme = "# awesome-quant\n\n## Trading & Backtesting\n"
head_readme = base_readme + "\n".join(
line[1:]
for line in FIVE_ENTRY_PATCH.splitlines()
if line.startswith("+- ")
) + "\n"
repository = FakeBaseRepository(
FakePull(10, files=changed_files),
base_readme=base_readme,
head_readme=head_readme,
)
client = FakeClient(repository)
stdout = io.StringIO()
stderr = io.StringIO()
environment = {
"GITHUB_TOKEN": "token",
"GITHUB_REPOSITORY": "owner/list",
"PR_NUMBER": "10",
}
with (
patch.dict("os.environ", environment, clear=True),
patch("sys.argv", ["review_pr.py"]),
patch("scripts.review_pr.Github", return_value=client),
redirect_stdout(stdout),
redirect_stderr(stderr),
):
result = main()
self.assertEqual(result, 0)
self.assertIn("Entries reviewed: 5", stdout.getvalue())
def test_skip_flag_is_reported_and_forwarded(self):
stdout = io.StringIO()
stderr = io.StringIO()
@@ -1016,7 +1094,7 @@ class MainTests(unittest.TestCase):
patch("scripts.review_pr.Github"),
patch(
"scripts.review_pr.review_pr",
return_value=([], "Add Fresh"),
return_value=([], "Add Fresh", 1),
) as reviewer,
redirect_stdout(stdout),
redirect_stderr(stderr),
@@ -1044,7 +1122,7 @@ class MainTests(unittest.TestCase):
with (
patch.dict("os.environ", environment, clear=True),
patch("sys.argv", ["review_pr.py"]),
patch("scripts.review_pr.review_pr", return_value=([], "Add Fresh")),
patch("scripts.review_pr.review_pr", return_value=([], "Add Fresh", 1)),
redirect_stdout(stdout),
redirect_stderr(stderr),
warnings.catch_warnings(),
@@ -1057,7 +1135,7 @@ class MainTests(unittest.TestCase):
def test_failure_reports_failed_check_and_nonzero_status(self):
finding = Finding("description", "PR body is empty")
result, stdout, _stderr = self.run_main(([finding], "Add Fresh"))
result, stdout, _stderr = self.run_main(([finding], "Add Fresh", 1))
self.assertEqual(result, 1)
self.assertIn("description: fail - PR body is empty", stdout)