daaain / daaain/claude-code-log
Add sandbox-based parallel test class for git_remote.resolve_sha
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 1.2k
- Forks
- 98
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 13
Description
Background
test/test_commit_linkifier.py::TestIntegrationLocalRepo validates the SHA resolver against the project's own git repo using 7c2e6f6 as a known historical SHA. This catches real-world things a mock couldn't — git CLI output format changes, GitHub URL canonicalization shifts, --contains behaviour — and is worth keeping for primary-CI smoke value.
But it has two gaps:
- Skips on forks. The
@pytest.mark.skipif(not _project_repo_origin_is_daaain_repo(), …)predicate means contributors with a forked remote get zero coverage of the resolver. - Brittle to repo evolution. The class needs
fetch-depth: 0in CI to reach historical SHAs (added in #161 as commit b3572b0). If main is ever force-pushed past7c2e6f6, or the remote moves, the test breaks.
Proposal
Add a parallel TestSandboxResolver class that runs everywhere (no skip), alongside the existing TestIntegrationLocalRepo. Setup per test method:
tempfile.TemporaryDirectory()→git inita fresh repo- Configure a fake remote:
git remote add origin git@github.com:owner/x.git(or HTTPS variant for URL-parser coverage) - Make 1–2 commits with deterministic content; capture the SHA
- Manually create the remote-tracking ref:
git update-ref refs/remotes/origin/main <sha>(avoids needing a realgit fetch) - Call
resolve_sha(tmpdir, short_sha)and assert URL shape
This exercises the resolver's full subprocess pipeline (git config, git branch -r --contains, git rev-parse --verify) without touching the network or depending on the project's own history.
Why not replace TestIntegrationLocalRepo?
The local-repo tests catch a different class of bug — they cross-check that the same code that works in a fabricated fixture also works against an organically-grown git repo with real refs, real remote URL, real history depth. Both layers are valuable: hermetic + smoke-against-reality.
Acceptance criteria
- New
TestSandboxResolverclass with at least: resolve happy-path, unresolved-SHA returns None, short-SHA expansion, URL-template selection (GitHub host), unknown-host returns None - Runs without skip on forks (no
@pytest.mark.skipif) - No new external deps; uses stdlib
tempfile+subprocess - CI still passes with both
TestIntegrationLocalRepoandTestSandboxResolverrunning
Out of scope (for this issue)
- Replacing or shrinking
TestIntegrationLocalRepo— keep it as the reality-check layer. - Reverting
fetch-depth: 0in.github/workflows/ci.yml. Once the sandbox class exists, that becomes optional, but flipping it should be a separate decision once we see how often the local-repo class catches things the sandbox doesn't.
Context: discussion on PR #161 review.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in test/test_commit_linkifier.py by reading TestIntegrationLocalRepo and the resolve_sha entry point, then run the existing resolver tests. Add the parallel TestSandboxResolver coverage described in the acceptance criteria using only tempfile and subprocess; done means all listed cases pass on forks and CI runs both test classes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- git, python
- Domain
- testing-qa
- Issue type
- Feature
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100