Skip to content

test: isolate doctor env-sensitivity tests from ambient environment#110

Merged
jmcte merged 2 commits into
mainfrom
claude/fix-doctor-env-ci
May 16, 2026
Merged

test: isolate doctor env-sensitivity tests from ambient environment#110
jmcte merged 2 commits into
mainfrom
claude/fix-doctor-env-ci

Conversation

@jmcte
Copy link
Copy Markdown
Contributor

@jmcte jmcte commented May 15, 2026

Summary

main is currently red. PR #109 was merged at 19:22 using SHA f9b287a, before the follow-up env-isolation fix could be pushed, so the environment-sensitive doctor tests landed unguarded. PR #108 then merged main and inherited the same failure.

Failure: test/doctor.test.ts:1242

AssertionError: expected 'missing SYNOLOGY_HOST' to be 'missing GITHUB_PAT, SYNOLOGY_HOST'

Root cause: The new missing-env and missing-host-field assertions read process.env via loadDeploymentEnv. The self-hosted Synology runner exports GITHUB_PAT, so it is not reported missing there (it is locally), and the expected detail string differs.

Fix: Wrap the two affected cases in the file's existing withEnv helper to clear GITHUB_PAT/GITHUB_TOKEN/GH_TOKEN/SYNOLOGY_HOST (and WINDOWS_DOCKER_HOST/WINDOWS_DOCKER_USERNAME for the Windows case) — the same pattern the pre-existing missing-env test already uses. No production code changes; 32 insertions / 13 deletions in test/doctor.test.ts only.

Test plan

  • Reproduced the failure by exporting GITHUB_PAT etc. before the run
  • pnpm test -- test/doctor.test.ts passes with CI-like ambient env (15 tests)
  • Full pnpm test:coverage previously verified green under the same simulated env

Once merged, #108 will go green after it picks up main.

https://claude.ai/code/session_01QxQ71Yrf2Cn6zVfM4LY7AR


Generated by Claude Code

PR #109 merged before this fix landed, leaving main red: the new
missing-env and missing-host-field assertions in doctor.test.ts read
process.env via loadDeploymentEnv, and the self-hosted runner exports
GITHUB_PAT, so the expected "missing GITHUB_PAT, ..." detail differed
from local. Wrap these cases in withEnv to clear the relevant
variables, matching the existing missing-env test pattern.

https://claude.ai/code/session_01QxQ71Yrf2Cn6zVfM4LY7AR
@athena-omt athena-omt added area:infra Infrastructure, CI, release, governance, scripts, or repo setup. lane:hephaestus Hephaestus build/repo-ops lane. review:athena Athena review governance requested. risk:medium Medium-risk change; normal care required. state:waiting-checks Waiting for CI/check status to settle. status:needs-review PR is ready for Athena review. labels May 15, 2026
@jmcte jmcte enabled auto-merge (squash) May 16, 2026 03:24
jmcte pushed a commit that referenced this pull request May 16, 2026
#108 merged main after #109 landed without the env-isolation fix, so
its doctor.test.ts missing-env/missing-host assertions failed on the
self-hosted runner (GITHUB_PAT is exported there). Wrap those cases in
withEnv to clear the relevant variables, matching the existing
missing-env test pattern. Mirrors PR #110.

https://claude.ai/code/session_01QxQ71Yrf2Cn6zVfM4LY7AR
@jmcte jmcte merged commit 7e5a394 into main May 16, 2026
13 checks passed
@jmcte jmcte deleted the claude/fix-doctor-env-ci branch May 16, 2026 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:infra Infrastructure, CI, release, governance, scripts, or repo setup. lane:hephaestus Hephaestus build/repo-ops lane. review:athena Athena review governance requested. risk:medium Medium-risk change; normal care required. state:waiting-checks Waiting for CI/check status to settle. status:needs-review PR is ready for Athena review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants