Skip to content

Commit 3775b59

Browse files
committed
spec: add patch verify command and surface remove-unimplemented-tests.sh in status
Two new capabilities that close gaps identified against CONTRIBUTING.md: - `dbx spec patch verify`: runs `git apply --check -R` on each patch individually and reports which are stale (file content has drifted since the patch was created) vs clean. Provides actionable guidance for both cases (remove the patch, or recreate it after a fresh sync). - `dbx spec status` now parses `.evergreen/remove-unimplemented-tests.sh` and shows a grouped summary of tracked exclusions by ticket. Inline and block-style ticket comments (PYTHON-XXXX / DRIVERS-XXXX) are both extracted. Stale entries whose source files no longer exist in the specs repo are flagged for removal. New spec files not covered by either a patch or the remove script are called out inline next to the stale-spec listing, making it clear when a new file needs to be handled.
1 parent f2f8807 commit 3775b59

1 file changed

Lines changed: 265 additions & 1 deletion

File tree

src/dbx_python_cli/commands/spec.py

Lines changed: 265 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Spec command for managing spec syncs with the MongoDB specifications repository."""
22

33
import filecmp
4+
import fnmatch
45
import os
56
import re
67
import subprocess
@@ -540,6 +541,77 @@ def _find_removable_patches(
540541
return removable
541542

542543

544+
def _parse_remove_script(
545+
script_path: Path,
546+
) -> list[tuple[str, str]]:
547+
"""Parse remove-unimplemented-tests.sh into [(driver_rel_path_glob, ticket), ...].
548+
549+
Handles:
550+
- ``rm $PYMONGO/test/foo.json # PYTHON-1234`` (inline ticket)
551+
- Block comments ``# PYTHON-5248 - description`` above rm commands
552+
- ``find /$PYMONGO/test -type f -name 'pre-42-*.json' -delete``
553+
"""
554+
try:
555+
text = script_path.read_text()
556+
except OSError:
557+
return []
558+
559+
entries: list[tuple[str, str]] = []
560+
pending_ticket = "" # ticket from most recent standalone comment line
561+
562+
for raw_line in text.splitlines():
563+
line = raw_line.strip()
564+
565+
if not line:
566+
# Blank line resets the pending block comment
567+
pending_ticket = ""
568+
continue
569+
570+
if line.startswith("#"):
571+
# Extract PYTHON-XXXX or DRIVERS-XXXX from standalone comment
572+
m = re.search(r"\b(PYTHON-\d+|DRIVERS-\d+)\b", line)
573+
pending_ticket = m.group(1) if m else ""
574+
continue
575+
576+
# Pull optional trailing inline comment (takes precedence over block comment)
577+
ticket = pending_ticket
578+
if " # " in line:
579+
line, comment_part = line.rsplit(" # ", 1)
580+
m = re.search(r"\b(PYTHON-\d+|DRIVERS-\d+)\b", comment_part)
581+
ticket = m.group(1) if m else comment_part.strip()
582+
583+
line = line.strip()
584+
585+
if line.startswith("rm "):
586+
path_str = line.split()[-1]
587+
path_str = re.sub(r"^/?\$\w+/", "", path_str)
588+
if path_str.startswith("test/"):
589+
entries.append((path_str, ticket))
590+
591+
elif "find " in line and "-name " in line:
592+
name_match = re.search(r"-name\s+'([^']+)'", line)
593+
dir_match = re.search(r"find\s+/?\$\w+/(\S+)", line)
594+
if name_match and dir_match:
595+
base_dir = dir_match.group(1).rstrip("/")
596+
pattern = name_match.group(1)
597+
path_str = f"{base_dir}/{pattern}"
598+
if path_str.startswith("test/"):
599+
entries.append((path_str, ticket))
600+
601+
return entries
602+
603+
604+
def _remove_script_tickets(entries: list[tuple[str, str]]) -> dict[str, list[str]]:
605+
"""Group remove-script entries by ticket → [path_glob, ...].
606+
607+
Entries without a ticket comment are grouped under an empty string key.
608+
"""
609+
result: dict[str, list[str]] = {}
610+
for path_glob, ticket in entries:
611+
result.setdefault(ticket, []).append(path_glob)
612+
return result
613+
614+
543615
def _spec_is_stale(
544616
specs_source: Path,
545617
driver_test: Path,
@@ -686,12 +758,17 @@ def spec_status(
686758
else:
687759
typer.echo(" No test/ files changed yet on this branch.")
688760

689-
# --- Parse spec map ---------------------------------------------------- #
761+
# --- Parse spec map and remove script ---------------------------------- #
690762
spec_map = _parse_resync_script(script)
691763
if not spec_map:
692764
typer.echo("\n⚠ Could not parse resync-specs.sh — no spec mappings found.")
693765
raise typer.Exit(1)
694766

767+
remove_script = driver_path / ".evergreen" / "remove-unimplemented-tests.sh"
768+
remove_entries = _parse_remove_script(remove_script)
769+
# Build a flat set of all glob patterns tracked by the remove script
770+
remove_globs: set[str] = {path_glob for path_glob, _ in remove_entries}
771+
695772
typer.echo(f"\n Checking {len(spec_map)} spec(s) against {specs_source}...\n")
696773

697774
stale: list[tuple[str, str]] = [] # (spec_name, reason)
@@ -727,6 +804,26 @@ def _patches_for_spec(spec_name: str) -> list[str]:
727804
tickets.append(t)
728805
return tickets
729806

807+
# Helper: check if a new-file reason is already tracked in the remove script
808+
def _new_files_covered(
809+
spec_name: str, mappings: list[tuple[str, str]]
810+
) -> list[str]:
811+
"""Return new spec filenames (relative to driver test/) NOT yet in remove script."""
812+
uncovered: list[str] = []
813+
for spec_dir, driver_dir in mappings:
814+
src = specs_source / spec_dir
815+
dst = driver_test / driver_dir
816+
if not src.exists() or not dst.exists():
817+
continue
818+
src_files = {f.relative_to(src) for f in src.rglob("*.json")}
819+
dst_files = {f.relative_to(dst) for f in dst.rglob("*.json")}
820+
for rel in src_files - dst_files:
821+
driver_rel = f"test/{driver_dir}/{rel}"
822+
# Check against remove_globs (exact match or fnmatch for wildcards)
823+
if not any(fnmatch.fnmatch(driver_rel, g) for g in remove_globs):
824+
uncovered.append(driver_rel)
825+
return sorted(uncovered)
826+
730827
# --- Output ------------------------------------------------------------ #
731828
if stale:
732829
typer.echo(f" ❌ Still needs syncing ({len(stale)}):\n")
@@ -737,6 +834,17 @@ def _patches_for_spec(spec_name: str) -> list[str]:
737834
tickets = _patches_for_spec(name)
738835
patch_str = f" 🩹 {', '.join(tickets)}" if tickets else ""
739836
typer.echo(f"{prefix} dbx spec sync {name}{reason_str}{patch_str}")
837+
if "new file" in reason:
838+
uncovered = _new_files_covered(name, spec_map.get(name, []))
839+
if uncovered:
840+
marker = " └──" if is_last else " │ "
841+
typer.echo(
842+
f"{marker}{len(uncovered)} new file(s) not yet in"
843+
" remove-unimplemented-tests.sh or a patch:"
844+
)
845+
if verbose:
846+
for f in uncovered:
847+
typer.echo(f"{marker} {f}")
740848
if any(_patches_for_spec(n) for n, _ in stale):
741849
typer.echo(
742850
"\n 🩹 = has an active patch — use --apply-patches to sync + patch in one shot"
@@ -779,6 +887,72 @@ def _patches_for_spec(spec_name: str) -> list[str]:
779887
for patch_path, _ in removable:
780888
typer.echo(f" rm {patch_path}")
781889

890+
# --- remove-unimplemented-tests.sh summary ----------------------------- #
891+
if remove_entries:
892+
remove_by_ticket = _remove_script_tickets(remove_entries)
893+
typer.echo(
894+
f"\n 🗂 remove-unimplemented-tests.sh"
895+
f" ({len(remove_entries)} tracked exclusion(s)"
896+
f" across {len(remove_by_ticket)} ticket(s)):"
897+
)
898+
for ticket, globs in sorted(remove_by_ticket.items()):
899+
label = ticket if ticket else "(no ticket)"
900+
subdirs = sorted(
901+
{
902+
g.split("/")[1]
903+
for g in globs
904+
if g.startswith("test/") and len(g.split("/")) >= 3
905+
}
906+
)
907+
dirs_str = f" → {', '.join(subdirs)}" if subdirs else ""
908+
if verbose:
909+
typer.echo(f" • {label} ({len(globs)} glob(s)){dirs_str}:")
910+
for g in globs:
911+
typer.echo(f" {g}")
912+
else:
913+
typer.echo(f" • {label} ({len(globs)} glob(s)){dirs_str}")
914+
915+
# Flag remove-script entries whose source files no longer exist in specs
916+
stale_removes: list[tuple[str, str]] = []
917+
for path_glob, ticket in remove_entries:
918+
parts = path_glob.split("/")
919+
if len(parts) < 3 or parts[0] != "test":
920+
continue
921+
driver_dir = parts[1]
922+
# Build matching specs source dir via spec_map
923+
src_dir: str | None = None
924+
for mappings in spec_map.values():
925+
for specs_src, drv_dst in mappings:
926+
if drv_dst == driver_dir:
927+
src_dir = specs_src
928+
break
929+
if src_dir:
930+
break
931+
if src_dir is None:
932+
continue
933+
rel = "/".join(parts[2:])
934+
src_base = specs_source / src_dir
935+
# Expand glob — if no matches in specs, flag as stale
936+
matches = (
937+
list(src_base.glob(rel))
938+
if "*" in rel or "?" in rel
939+
else ([src_base / rel] if (src_base / rel).exists() else [])
940+
)
941+
if not matches:
942+
stale_removes.append((path_glob, ticket))
943+
944+
if stale_removes:
945+
typer.echo(
946+
f"\n 🗑 {len(stale_removes)} remove-script entry/entries may be"
947+
" removable (source no longer in specs repo):"
948+
)
949+
for path_glob, ticket in stale_removes:
950+
label = f" [{ticket}]" if ticket else ""
951+
typer.echo(f" • {path_glob}{label}")
952+
typer.echo(
953+
"\n ℹ Run 'dbx spec patch verify' to check whether patch files still apply cleanly."
954+
)
955+
782956
# --- What to do next --------------------------------------------------- #
783957
typer.echo("\n 🔍 To verify locally:\n")
784958
step = 1
@@ -1129,3 +1303,93 @@ def patch_apply(
11291303
if not _apply_patches(driver_repo, verbose):
11301304
raise typer.Exit(1)
11311305
typer.echo("✅ All patches applied.")
1306+
1307+
1308+
# ---------------------------------------------------------------------------
1309+
# dbx spec patch verify
1310+
# ---------------------------------------------------------------------------
1311+
1312+
1313+
@patch_app.command("verify")
1314+
def patch_verify(
1315+
ctx: typer.Context,
1316+
repo_name: str = typer.Option(
1317+
"mongo-python-driver",
1318+
"--repo",
1319+
"-r",
1320+
help="Driver repository to verify patches in",
1321+
),
1322+
):
1323+
"""Check whether all patch files still apply cleanly.
1324+
1325+
Runs ``git apply --check -R`` on each patch individually and reports
1326+
which are valid and which are stale (no longer match the current file
1327+
content). A stale patch means the spec file was changed by something
1328+
other than the patch — either a later spec sync updated the file in a
1329+
way that the patch didn't anticipate, or someone edited it directly.
1330+
1331+
Usage::
1332+
1333+
dbx spec patch verify
1334+
dbx spec patch verify -r django-mongodb-backend
1335+
"""
1336+
verbose = ctx.obj.get("verbose", False) if ctx.obj else False
1337+
config = get_config()
1338+
base_dir = get_base_dir(config)
1339+
1340+
driver_repo = _get_driver_repo(repo_name, base_dir, config)
1341+
patch_dir = _get_patch_dir(driver_repo)
1342+
patches = _list_patches(patch_dir)
1343+
1344+
if not patches:
1345+
typer.echo(f"No patch files found in {patch_dir}")
1346+
return
1347+
1348+
typer.echo(f"\n🔍 Verifying {len(patches)} patch(es) in {repo_name}:\n")
1349+
1350+
ok: list[str] = []
1351+
stale: list[tuple[str, str]] = [] # (ticket, error_msg)
1352+
1353+
for patch_path in patches:
1354+
ticket = patch_path.stem
1355+
result = subprocess.run(
1356+
["git", "apply", "--check", "-R", "--allow-empty", str(patch_path)],
1357+
cwd=str(driver_repo["path"]),
1358+
check=False,
1359+
capture_output=True,
1360+
text=True,
1361+
)
1362+
if result.returncode == 0:
1363+
ok.append(ticket)
1364+
typer.echo(f" ✅ {ticket}")
1365+
else:
1366+
err = result.stderr.strip()
1367+
stale.append((ticket, err))
1368+
typer.echo(f" ❌ {ticket} [stale — does not apply cleanly]")
1369+
if verbose:
1370+
for line in err.splitlines():
1371+
typer.echo(f" {line}")
1372+
1373+
typer.echo("")
1374+
if stale:
1375+
typer.echo(
1376+
f" {len(stale)} stale patch(es) found. The spec file(s) they target"
1377+
" have drifted since the patch was created.\n"
1378+
)
1379+
typer.echo(" For each stale patch, you have two options:")
1380+
typer.echo(
1381+
" a) The spec now matches what the driver does → remove the patch:"
1382+
)
1383+
for ticket, _ in stale:
1384+
typer.echo(f" dbx spec patch remove {ticket}")
1385+
typer.echo(
1386+
" b) The spec changed but driver still unimplemented → recreate the patch:"
1387+
)
1388+
typer.echo(" dbx spec sync <spec-name>")
1389+
typer.echo(" # edit the files to reflect driver behavior")
1390+
typer.echo(
1391+
" dbx spec patch remove <ticket> && dbx spec patch create <ticket>"
1392+
)
1393+
raise typer.Exit(1)
1394+
else:
1395+
typer.echo(f" All {len(ok)} patch(es) apply cleanly.")

0 commit comments

Comments
 (0)