test(scaffold): add full first-commit e2e regression test#310
test(scaffold): add full first-commit e2e regression test#310ichoosetoaccept merged 3 commits intomainfrom
Conversation
|
This change is part of the following stack:
Change managed by git-spice. |
Greptile SummaryAdds Confidence Score: 5/5Safe to merge — the new test is logically correct and the only finding is a P2 style suggestion about subprocess timeouts. All changes are confined to a single test method. The logic is sound: SKIP is correctly stripped from git_env, the two-attempt retry mirrors the real user flow accurately, and the assertions correctly distinguish first-attempt hook auto-fixes from genuine failures. The one open finding (no timeout on network-bound subprocess calls) is a reliability nicety, not a correctness issue, and consistent with the pattern used by the adjacent test_uv_sync_succeeds. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Test as test_first_commit_succeeds
participant Copier as copier copy -r HEAD
participant UV as uv sync
participant Git as git init --initial-branch=main
participant Prek as uv run prek install
participant AutoUpdate as bash scripts/prek-autoupdate.sh
participant Commit as git commit (attempt N)
Test->>Copier: generate_project(tmp_path, copier_defaults)
Copier-->>Test: project dir
Test->>UV: uv sync
UV-->>Test: assert returncode==0
Test->>Git: git init
Git-->>Test: ok
Test->>Prek: prek install
Prek-->>Test: assert returncode==0
Test->>AutoUpdate: fetch latest hook revisions
AutoUpdate-->>Test: assert returncode==0
loop up to 2 attempts
Test->>Test: git add -A
Test->>Commit: git commit (env=git_env, SKIP removed)
alt returncode==0
Commit-->>Test: break — success
else attempt==1 and returncode!=0
Commit-->>Test: hooks auto-fixed files, re-stage and retry
else attempt==2 and returncode!=0
Commit-->>Test: assert attempt==1 → FAIL with stdout/stderr
end
end
Prompt To Fix All With AIThis is a comment left during a code review.
Path: tests/test_template.py
Line: 1300-1335
Comment:
**No subprocess timeout on network-bound steps**
`uv sync`, `prek install`, and especially `bash scripts/prek-autoupdate.sh` (which fetches the latest revision for every hook repo from GitHub) can hang indefinitely if the network is slow or a host is unresponsive. Without a `timeout=` argument, `subprocess.run` will wait forever, stalling the CI runner until the job-level watchdog kills it — with no signal about which step hung.
Consider adding a generous timeout to each call (e.g. 300 s for `prek-autoupdate.sh` and `uv sync`, since they're already gated behind `@pytest.mark.slow`):
```python
autoupdate = subprocess.run(
["bash", "scripts/prek-autoupdate.sh"],
cwd=project,
capture_output=True,
text=True,
check=False,
timeout=300,
)
```
Same pattern applies to `uv sync` (line 1300) and `prek install` (line 1320).
How can I resolve this? If you propose a fix, please make it concise.Reviews (3): Last reviewed commit: "test(scaffold): add full first-commit e2..." | Re-trigger Greptile |
6e70c0a to
829e28b
Compare
22886f7 to
003cabb
Compare
…h-protection docs link
The lychee hook was 404'ing on a fresh scaffold for URLs that point at the
user's own repo (which doesn't exist on GitHub yet) and at the stale
`managing-a-branch-protection-rule` docs page (GitHub renamed the path).
Adds three exclusion patterns to `.lychee.toml.jinja`:
- `{repository_host}/{repository_namespace}/{repository_name}` -- covers
workflow badges, action queries, discussions URL, etc.
- `{repository_namespace}.github.io/{repository_name}` -- covers the
GitHub Pages docs site that doesn't exist before first deploy.
- `pypi.org/project/{distribution_name}` -- only generated when the
project plans to publish, but won't exist before first release.
These are user-owned URLs -- lychee can't tell us anything we don't already
know, and they break every fresh-scaffold first commit.
Updates the README's branch protection link to a current docs page.
Generates a fresh project, runs uv sync, git init, prek install, the autoupdate workaround script, then attempts the first commit. Mirrors the real user flow: prek hooks may auto-fix files (formatters, lockfile sync) on the first run, abort the commit, and the user re-stages and retries. The test allows up to 2 attempts; the second must succeed. Catches regressions in DOT-491 (pytest-testmon exit 5 on empty test dir), DOT-492 (lychee install rejection after autoupdate), and the lychee URL 404s on a fresh scaffold.
829e28b to
947bada
Compare
003cabb to
55188d3
Compare
Refs DOT-491
Refs DOT-492
Refs DOT-493
Refs DOT-494
Summary
test_first_commit_succeeds_with_prek_hookstotests/test_template.py. Generates a fresh project, runsuv sync,git init,prek install,bash scripts/prek-autoupdate.sh, then attempts the first commit.Why
This is the single regression test that guards every fresh-scaffold bug we've fixed in this stack. Without it, future template changes can silently re-break the first-commit experience and we'd never know until a user complains.
Runs with no
SKIP=— every prek hook on every freshly scaffolded file. Marked@pytest.mark.slowso it only runs inpoe test-all/poe test-cov, not the fast loop.Test plan
main— confirmed each fix is load-bearing