test(ssh): cover helper error classification - #1073
Open
devsy-app[bot] wants to merge 3 commits into
Open
Conversation
Extract isSignalInterrupt pure function from handleRunError (behavior-preserving) and add helper_test.go covering ExitError, RunOptions.validate, isSignalInterrupt, handleRunError, and setupContextCancellation.
✅ Deploy Preview for images-devsy-sh canceled.
|
✅ Deploy Preview for devsydev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 16 |
| Duplication | 0 |
AI Reviewer: run a review on demand. To trigger the first review automatically, go to your organization or repository integration settings. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
Signed-off-by: Samuel K <skevetter@pm.me>
skevetter
marked this pull request as ready for review
August 17, 2026 07:05
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.
Package reviewed
pkg/ssh(helper.go) - SSH/git/credentials category.Issue found
helper.gohad no test file. Its core helpers were entirely untested:ExitError(the package's exit-status error type:Error(),Unwrap())RunOptions.validate(client/command precondition checks)handleRunError(error classification: cancelled context ->ctx.Err(),io.EOF-> wrapped, generic -> wrapped,*ssh.ExitError-> wrapped inExitError)setupContextCancellation(context watcher lifecycle + already-cancelled fast path)The most subtle logic - the signal-exit-code classification inside
handleRunError(exit codes130/129/143=>nil, i.e. SIGINT/SIGHUP/SIGTERM are treated as normal interactive-session exits) - was inline and unreachable by a direct unit test, because*ssh.ExitErrorembeds the unexportedssh.Waitmsgand cannot be constructed from outside the package with a custom exit status.Change
One behavior-preserving refactor + tests (no behavioral change):
pkg/ssh/helper.go- extracted the inline130 || 129 || 143check into a small pure functionisSignalInterrupt(exitCode int) bool, called from the same spot inhandleRunError. The classification semantics are identical; only the shape changed so the logic is independently testable.pkg/ssh/helper_test.go(new) - 12 focused tests:ExitError.Error()with/without a wrapped err;Unwrap()round-trips viaerrors.Is/errors.Unwrap.RunOptions.validaterejects missing client, rejects missing command, accepts both present.isSignalInterrupt:130/129/143=> true;0/1/2/127/128/131/142/144/255/-1=> false.handleRunError: cancelled context returnscontext.Canceled;io.EOFis wrapped with the command and stayserrors.Is(io.EOF); a generic error is wrapped and stayserrors.Is(inner); a*ssh.ExitError(zeroWaitmsg=>ExitStatus()==0, not a signal interrupt) is wrapped in ourExitErrorwith the right code and preserveserrors.Is(sshExitErr).setupContextCancellation: already-cancelled context returnscontext.Canceledwith nil cleanup; a fresh context returns a non-nil cleanup that is safe to call once.Verification
task cli:format- clean.task cli:lint:ci- 0 issues (new-from-patch against origin/main).task cli:test-pkg/sshpasses (ok github.com/devsy-org/devsy/pkg/ssh, coverage 51.2%). The only failure ispkg/gitTestRepoClone*, which is the known pre-existing failure on origin/main (confirmed by checking out origin/main and reproducing the identical failures) and is unrelated to this change (touchespkg/sshonly).go test ./pkg/ssh/(focused) - all 12 new tests pass.This PR was created by an AI agent as part of an automated daily package review job.