Fix integration test compile break left by PTY migration - #453
Merged
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
skyrpex
marked this pull request as draft
August 12, 2026 15:03
skyrpex
marked this pull request as ready for review
August 12, 2026 15:05
There was a problem hiding this comment.
LGTM 🎉 — finishes the #444 xpty-helper migration for the one missed test, and I confirmed the test/integration module now compiles (go vet ./... → 0) and is gofmt-clean (gofmt -l → empty), with no behavior change beyond re-enabling the test where it already ran. @skyrpex
Automated review on behalf of @gtsiolis.
Generated by Claude Code
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.
Motivation
origin/main'stest/integrationmodule does not compile since #444: that PR removed thebytes,runtime, andgithub.com/creack/ptyimports fromlicense_test.goand migratedTestLicenseRejectionOffersReloginAndRetriesto the xpty helpers, but leftTestLicenseRejectionEscDeclineShowsManualStepsstill callingpty.Start/runtime.GOOS/bytes.Contains— undefined identifiers, so the integration lint/test CI jobs on main are red. The same squash also landed five files that are not gofmt-clean.How this got past CI: #444's last commit was pushed before #449 merged, and #449 added
TestLicenseRejectionEscDeclineShowsManualSteps— a new test using exactly the imports #444 was removing. The two changes touch different regions oflicense_test.go, so the squash merged cleanly with no textual conflict, and the branch was merged without rebasing or re-running CI — each PR was green individually, but nothing ever compiled the combination until it landed on main. (Requiring up-to-date branches, or a merge queue, would prevent this class of breakage.)Solution
Finish the #444 migration for the one missed test:
TestLicenseRejectionEscDeclineShowsManualStepsnow usesstartLstkInPTY/waitForOutputTimeout/write/waitexactly like its already-migrated sibling, the Windows PTY skip is dropped (the helpers are ConPTY-capable;requireDockerstill gates it on Windows CI), and the transcript-on-failure logging is kept viap.output(). Alsogofmt -won the five unformatted files (env/env.go,logout_test.go,logs_test.go,status_test.go,volume_test.go) — whitespace only.Docs
Nothing to document: test-only compile fix and formatting, no user-facing behavior change.
Review
Self-merge candidate: restores compilation with the exact helper pattern #444 already established (and that the sibling test uses); no behavior change beyond re-enabling the test on the platforms it already targeted. Verified with
go vet,gofmt, andgolangci-lint(0 issues) in thetest/integrationmodule.Co-Authored-By: Claude noreply@anthropic.com
🤖 Generated with Claude Code