Repository navigation
fix(connections): restore the test environment in place so SSH tests pass on Node 26 - #328
Merged
Merged
Conversation
…pass on Node 26 Nineteen SSH tests failed on Node 26 and passed on Node 22. After each test, the three files assigned a copy to process.env, and from Node 26 child processes do not see variables set on an assigned object. The fake ssh stopped receiving its FAKE_SSH_* settings, so a test expecting a failure could get a successful connection. A small helper now restores the environment in place, which works on every version. Production code never assigns to process.env, so only the tests change.
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
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.
Nineteen SSH connection tests failed on Node 26, which
enginesallows and this machine uses by default, while passing on CI's Node 22. A developer on Node 26 saw red tests with misleading errors, and three earlier PR bodies blamed them on the machine's SSH keys. The tests now pass on both versions. Users see no change, because production code never assigns toprocess.env.tests/electron/connections.test.ts,ssh-test.test.ts, andssh-tunnel.test.tsranprocess.env = { ...environment }. From Node 26, child processes do not see variables set on an assigned object, sotests/fixtures/fake-ssh.cjsstopped receiving itsFAKE_SSH_*settings. The first test in a file still passed, since nothing had been assigned yet.environmentRestorerintests/helpers/restore-environment.ts, which removes added variables and restores the original values on the same object. The three files now use it.Testing
npm run lint: clean (two pre-existing warnings insrc/components/storage-*.tsx, untouched).npm run format:check,npm run typecheck(all four tsconfigs), andnpm run build: clean.npm test: 2443 passed on Node 22.22.2 and on Node 26.13.0. Before this change, Node 26 failed 19 of them. No new tests; the 19 are the check.process.env, a child process sees the new variable on Node 22 and 24, and not on Node 26.Not in this PR
CI still runs only Node 22. Adding Node 26 would cost about three more minutes on every PR, so it is left out for now.