Skip to content

CI: shared-files test no longer depends on GITHUB_REPOSITORY - #217

Merged
bernardladenthin merged 1 commit into
mainfrom
claude/hopeful-pascal-9jlbqb
Oct 7, 2026
Merged

bernardladenthin merged 1 commit into
mainfrom
claude/hopeful-pascal-9jlbqb

Conversation

@bernardladenthin

Copy link
Copy Markdown
Owner

Summary

  • Fixes test_sibling_differences_are_warnings_not_failures in the shared .github/buildcheck/tests/test_sharedfiles.py. Since it was added on 2026-09-30, it has been red in BitcoinAddressFinder's CI only: Shared files + build checks, for example run 37680715063.
  • Cause: main() never fetches the repository being checked, and it takes that name from GITHUB_REPOSITORY. The test expects the sibling it leaves unreachable, BitcoinAddressFinder, to produce a could not read … warning. In BitcoinAddressFinder's own CI that sibling is the current repository and is skipped, so the warning never appeared. Locally (GITHUB_REPOSITORY unset) and in the other three repos the test passed.
  • Fix: the test pins current_repo to java-llama.cpp, the same way the other tests in the file already mock it, and additionally asserts that the pinned repository is not fetched.
  • It is a shared file: the copy is byte-identical in all four repos, and every manifest hash is refreshed.

Test plan

  • Reproduced: GITHUB_REPOSITORY=x/BitcoinAddressFinder python3 -m unittest discover -s .github/buildcheck/tests -t .github fails before the fix and passes after it.
  • In each repo the full buildcheck suite passes under its own repository name.
  • check-shared-files.py reports 0 files changed here alone, with the same hash in all four repos.
  • CI is green on this branch.

Related issues / PRs

The same change is made in java-llama.cpp, BitcoinAddressFinder, streambuffer and srcmorph.

Checklist

  • I have read CONTRIBUTING.md and CODE_OF_CONDUCT.md
  • My commits follow Conventional Commits. They use the repository's usual CI: prefix instead.
  • No security-sensitive changes

🤖 Generated with Claude Code

https://claude.ai/code/session_01AytmJF9faEiQEVt6eetQS2


Generated by Claude Code

test_sibling_differences_are_warnings_not_failures expected a
"could not read BitcoinAddressFinder" warning from the sibling it leaves
unreachable. main() never fetches the repository being checked, and takes
its name from GITHUB_REPOSITORY, so in BitcoinAddressFinder's own CI that
sibling was skipped and the test failed there alone (red since the test
was added; e.g. run 37680715063). The test now pins current_repo, as the
other tests in the file do, and asserts the pinned repository is not
fetched. Shared file: identical in all four repos, manifests refreshed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AytmJF9faEiQEVt6eetQS2
@claude

claude Bot commented Oct 7, 2026

Copy link
Copy Markdown

✅ Review approved

This PR successfully fixes the flaky test that was failing in BitcoinAddressFinder's CI. The fix correctly isolates the test from the GITHUB_REPOSITORY environment variable by pinning current_repo via mock.patch.object(), matching patterns already established in RepoEntryTest.

Key observations:

  • mock module properly imported (line 10)
  • Mocking pattern consistent with existing tests
  • Defensive assertion added (assertNotIn java-llama.cpp) strengthens test verification
  • SHA-256 hash verified and correct
  • Comment explains the issue context clearly

No issues found — code quality, security, and correctness are all solid.

@bernardladenthin
bernardladenthin merged commit 76f6afa into main Oct 7, 2026
20 of 28 checks passed
@bernardladenthin
bernardladenthin deleted the claude/hopeful-pascal-9jlbqb branch October 7, 2026 21:34

This branch had an error being deployed

1 failed and 1 active deployments
startgate — e3c196d7 Deployed Oct 7, 2026 by bernardladenthin via Start gate (abort window) #381
maven-central — e3c196d7 Deployed Oct 7, 2026 by bernardladenthin via Verify GPG signing key (no secrets printed) #381
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants