daaain / daaain/claude-code-log

Add sandbox-based parallel test class for git_remote.resolve_sha

Open
#162 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

cleanup
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:

  1. 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.
  2. Brittle to repo evolution. The class needs fetch-depth: 0 in CI to reach historical SHAs (added in #161 as commit b3572b0). If main is ever force-pushed past 7c2e6f6, 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:

  1. tempfile.TemporaryDirectory()git init a fresh repo
  2. Configure a fake remote: git remote add origin git@github.com:owner/x.git (or HTTPS variant for URL-parser coverage)
  3. Make 1–2 commits with deterministic content; capture the SHA
  4. Manually create the remote-tracking ref: git update-ref refs/remotes/origin/main <sha> (avoids needing a real git fetch)
  5. 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 TestSandboxResolver class 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 TestIntegrationLocalRepo and TestSandboxResolver running

Out of scope (for this issue)

  • Replacing or shrinking TestIntegrationLocalRepo — keep it as the reality-check layer.
  • Reverting fetch-depth: 0 in .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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.