rogpeppe
testscript: add pty/ptyout commands
This allows testing commands that open /dev/tty to interact with the user separately from stdin/stdout, like age.
See https://github.com/FiloSottile/age/blob/084c974f5393e5d2776fb1bb3a35eeed271a32fa/cmd/age/testdata/terminal.txt and https://github.com/FiloSottile/age/blob/084c974f5393e5d2776fb1bb3a35eeed271a32fa/cmd/age/testdata/scrypt.txt for example scripts.
I'm slightly worried about adding a hard dependency on creack/pty, which is not a small library. I understand that testscript is for testing CLIs and this is an important feature to support, but I imagine it would be nicer to allow plugging in any PTY library. That would be more flexible long-term, and would keep the amount of dependencies small for the majority of users who don't need this feature.
Pluggable PTY libraries sounds complex to me, and they have a relatively simple job: opening a pair of file descriptors that work as a virtual terminal. If that's the only blocker, I can probably reimplement the ~40% of creack/pty we need, especially if you're only targeting macOS and Linux.
I have historically only used creack/pty on unix-y systems, but I'm not sure if trying to use it on Windows makes sense :)
To be clear, what I mean by pluggable is to keep your patch as-is, but abstract away the Open, Write, and Close calls to an interface that's satisfied by the library. That doesn't sound particularly complex to me, and gives the downstream the ability to choose which libraries to incorporate. As you can see we've been very careful about adding dependencies, and we're even trying to get rid of pkg/diff soon. The only dependency I think we'll want to add in the near term is x/tools, for the sake of forwarding now-redundant packages like txtar.
Rebased and removed the creack/pty dependency. Opening a PTY pair is really just a OpenFile and a couple IOCTLs.
Test added! Testing -stdin would require a dependency on x/term or more IOCTLs and it didn't feel worth it.
Sweet, thank you for the reviews! Resolved the conflict, should be ready to merge.
@FiloSottile you got a 10m pty testscript timeout on macos on 1.20 there, any idea what might have happened? it succeeded on macos on 1.19, so perhaps it's a race or flakiness of some sort.
@FiloSottile you got a 10m pty testscript timeout on macos on 1.20 there, any idea what might have happened? it succeeded on macos on 1.19, so perhaps it's a race or flakiness of some sort.
Turns out it's a os Go 1.20 regression. golang/go#61779
Rebased and added a skip for https://github.com/golang/go/issues/61779.