<!--
If you have used AI to produce some or all of this PR, please ensure you have read our [AI Contribution guidelines](https://coder.com/docs/about/contributing/AI_CONTRIBUTING) before submitting.
-->Part of https://github.com/coder/internal/issues/1400
Refactors CLI tests of the `create` command as the first batch of tests refactored to take a PTY out of the loop.
One interesting difference I noticed between PTY and a direct pipe to standard in is that on the PTY we write `\r` to enter some input, but the kernel actually sends `\n` (or maybe `\r\n`) to the process, at least on Unix. (On windows we sent `\r\n` into the PTY). This is reflected in the implementation of the `Writer` , otherwise mostly inspired by the PTYTest equivalents.
Relates to https://github.com/coder/internal/issues/1400
Extracts the code that matches command output from the code that sets up a PTY, so it can be used independently.
Subsequent PRs will actually refactor the tests to use this directly over an inmemory pipe.<!--
If you have used AI to produce some or all of this PR, please ensure you have read our [AI Contribution guidelines](https://coder.com/docs/about/contributing/AI_CONTRIBUTING) before submitting.
-->
closes: https://github.com/coder/internal/issues/1331
Fixes up an issue in the test where we end up calling `FailNow` outside
the main test goroutine. Also adds the ability to name a `ptytest.PTY`
for cases like this one where we start multiple commands. This will help
debugging if we see the issue again.
This doesn't address the root cause of the failure, but I think we
should close the flake issue. I think we'd need like a stacktrace of all
goroutines at the point of failing the test, but that's way too much
effort unless we see this again.
On Windows, `pty.New()` was creating a `ConPTY` (`PseudoConsole`) even
when no process would be attached. `ConPTY` requires a real process to
function correctly - without one, the pipe handles become invalid
intermittently, causing flaky test failures like `read |0: The handle is
invalid.`
This affected tests using the `ptytest.New()` + `Attach()` pattern for
in-process CLI testing.
The fix splits Windows PTY creation into two paths:
- `newPty()` now returns a simple pipe-based PTY for the `Attach()` use
case
- `newConPty()` creates a real `ConPTY`, called by `Start()` when a
process will be attached
AFAICT this will result in no change in behaviour outside of tests.
Fixescoder/internal#1277
_Disclaimer: investigated and implemented by Claude Opus 4.5, reviewed
by me._
---------
Signed-off-by: Danny Kopping <danny@coder.com>
Closes https://github.com/coder/internal/issues/1173,
https://github.com/coder/internal/issues/1174
Currently these two tests are flaky because the contexts were created
before a potentially long-running process. By the time the context was
actually used, it may have timed out - leading to confusion.
Additionally, the `ExpectMatch` calls were not using the test context -
but rather a background context. I've marked that func as deprecated
because we should always tie these to the test context.
Special thanks to @mafredri for the brain probe 🧠
---------
Signed-off-by: Danny Kopping <danny@coder.com>
- Update go.mod to use Go 1.24.1
- Update GitHub Actions setup-go action to use Go 1.24.1
- Fix linting issues with golangci-lint by:
- Updating to golangci-lint v1.57.1 (more compatible with Go 1.24.1)
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <claude@anthropic.com>
The experimental functions in `golang.org/x/exp/slices` are now
available in the standard library since Go 1.21.
Reference: https://go.dev/doc/go1.21#slices
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Fixes a race in TestUpdateValidateRichParameters where the parameter is sent prior to the prompt.
Causes errors like: https://github.com/coder/coder/actions/runs/11681622439/job/32527173007
```
ptytest.go:132: 2024-11-05 10:02:18.819: cmd: "bool_parameter"
ptytest.go:167: 2024-11-05 10:02:18.819: cmd: matched "bool_parameter" = "bool_parameter"
update_test.go:440: 2024-11-05 10:02:18.819: cmd: stdin: "cat\r\n"
ptytest.go:132: 2024-11-05 10:02:18.819: cmd: "> Enter a value (default: \"\"): can't validate build parameter \"bool_parameter\": boolean value can be either \"true\" or \"false\""
ptytest.go:167: 2024-11-05 10:02:18.819: cmd: matched "boolean value can be either" = "\n> Enter a value (default: \"\"): can't validate build parameter \"bool_parameter\": boolean value can be either"
update_test.go:440: 2024-11-05 10:02:18.819: cmd: stdin: "\r\n"
ptytest.go:167: 2024-11-05 10:02:18.819: cmd: matched "Enter a value" = " \"true\" or \"false\"\n> Enter a value"
update_test.go:440: 2024-11-05 10:02:18.819: cmd: stdin: "false\r\n"
ptytest.go:132: 2024-11-05 10:02:18.821: cmd: "> Enter a value (default: \"\"): can't validate build parameter \"bool_parameter\": boolean value can be either \"true\" or \"false\""
```
* chore: add /v2 to import module path
go mod requires semantic versioning with versions greater than 1.x
This was a mechanical update by running:
```
go install github.com/marwan-at-work/mod/cmd/mod@latest
mod upgrade
```
Migrate generated files to import /v2
* Fix gen
Writing to stdin for `coder ssh` too early could result in the input
being discarded. To work around this we add a new `ptytest` method
called `ReadRune` that lets us read one character of output. This will
indicate the command is ready to accept input.
It could be one character of the prompt, or of the loading message
waiting for connection to be established.
To ensure ptytest closure always happens the same way, we now define a
new `Close` function on `PTY` and always call the close function instead
of manually closing read/writers.
A few additional log messages have been added as well, to better
understand the shutdown process in case of errors.
There are sporadic flakes in some tests on Windows related to the use of
`ptytest`. Since it's not clear why they fail, this commit adds some
more logging and timeouts to the cleanup methods.
Since we were not failing tests with `require` the error output was
somewhat hidden in the stream of log messages. This change standardizes
`ptytest` logging and failing to improve visibility.
* feat: Add workspace agent for SSH
This adds the initial agent that supports TTY
and execution over SSH. It functions across MacOS,
Windows, and Linux.
This does not handle the coderd interaction yet,
but does setup a simple path forward.
* Fix pty tests on Windows
* Fix log race
* Lock around dial error to fix log output
* Fix context return early
* fix: Leaking yamux session after HTTP handler is closed
Closes#317. We depended on the context canceling the yamux connection,
but this isn't a sync operation. Explicitly calling close ensures the
handler waits for yamux to complete before exit.
* Lock around close return
* Force failure with log
* Fix failed handler
* Upgrade dep
* Fix defer inside loops
* Fix context cancel for HTTP requests
* Fix resize
* Initial agent
* fix: Use buffered reader in peer to fix ShortBuffer
This prevents a io.ErrShortBuffer from occurring when the byte
slice being read is smaller than the chunks sent from the opposite
pipe.
This makes sense for unordered connections, where transmission is
not guarunteed, but does not make sense for TCP-like connections.
We use a bufio.Reader when ordered to ensure data isn't lost.
* SSH server works!
* Start Windows support
* Something works
* Refactor pty package to support Windows spawn
* SSH server now works on Windows
* Fix non-Windows
* Fix Linux PTY render
* FIx linux build tests
* Remove agent and wintest
* Add test for Windows resize
* Fix linting errors
* Add Windows environment variables
* Add strings import
* Add comment for attrs
* Add goleak
* Add require import
Spike Curtis