test: isolate doctor env-sensitivity tests from ambient environment#110
Open
jmcte wants to merge 1 commit into
Open
test: isolate doctor env-sensitivity tests from ambient environment#110jmcte wants to merge 1 commit into
jmcte wants to merge 1 commit into
Conversation
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
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mainis currently red. PR #109 was merged at 19:22 using SHAf9b287a, 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:1242Root cause: The new missing-env and missing-host-field assertions read
process.envvialoadDeploymentEnv. The self-hosted Synology runner exportsGITHUB_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
withEnvhelper to clearGITHUB_PAT/GITHUB_TOKEN/GH_TOKEN/SYNOLOGY_HOST(andWINDOWS_DOCKER_HOST/WINDOWS_DOCKER_USERNAMEfor the Windows case) — the same pattern the pre-existing missing-env test already uses. No production code changes; 32 insertions / 13 deletions intest/doctor.test.tsonly.Test plan
GITHUB_PATetc. before the runpnpm test -- test/doctor.test.tspasses with CI-like ambient env (15 tests)pnpm test:coveragepreviously verified green under the same simulated envOnce merged, #108 will go green after it picks up main.
https://claude.ai/code/session_01QxQ71Yrf2Cn6zVfM4LY7AR
Generated by Claude Code