Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Future of testscript given rsc.io/script and rsc.io/script/scripttest #238

Closed
twpayne opened this issue Nov 5, 2023 · 13 comments
Closed

Comments

@twpayne
Copy link
Contributor

twpayne commented Nov 5, 2023

So rsc.io/script and rsc.io/script/scripttest just go published: effectively a semi-official extraction of a more recent version of testscript from the Go source code.

Should we move there?

@twpayne
Copy link
Contributor Author

twpayne commented Nov 5, 2023

Note that rsc.io/script currently requires Go 1.22, so I guess there's no rush to switch.

@ldemailly
Copy link

so kinda like #196 yet not

@mvdan
Copy link
Collaborator

mvdan commented Nov 6, 2023

I've had it somewhere on my radar to try to rewrite our existing public API in terms of a direct copy of cmd/go/internal/script, which would significantly reduce the maintenance load for us. It would be somewhat tricky to fit the two together though, as they have diverged significantly over time - we froze the API and added features on top, and they basically rewrote their API entirely to be more reusable.

Honestly, if they commit to maintaining a public version of their testing script engine, I'm totally in favor of using that and no longer having to maintain a fork :) The two tricky bits will be how easy it might be to rewrite existing tests to use that new module, and how many features might one miss from testscript that could or should be ported over.

@twpayne
Copy link
Contributor Author

twpayne commented Nov 6, 2023

Thanks for the honest response!

I (as a heavy and extremely grateful user of testscript) have also compared the current rsc.io/script API to the testscript API and realized that it will require a significant refactor of to migrate. However, the changes will be almost exclusively in the test infrastructure, not the tests themselves. We also have the delightful upside of having a whole bunch of tests that already pass, so we can declare our port to rsc.io/script as "done" once our existing tests pass.

So, it's probably ~small-N days of work to migrate, and this way we don't have to maintain a fully-diverged fork. I, for one, would be happy to help migrate the multiple improvements in testscript to our new rsc.io/script overlords.

@mvdan
Copy link
Collaborator

mvdan commented Nov 6, 2023

I skimmed the API a bit and tried to use it in a small project. Splitting the script engine out of the testing package is nice, it makes the library more generic. Some notes from trying to switch over:

  • There appears to be no support for setting up programs in $PATH, which is really useful for developing tools that get executed indirectly - I rely on this from the current package.
  • There's no built-in filtering of the environment like what the current package does, so the only easy option is to use os.Environ(). There are pros and cons, but one particular downside of passing the entire env is that tests easily change behavior depending on the machine.
  • I rely on UpdateScripts very regularly, which isn't there. Also RequireUniqueNames and ContinueOnError.

None of these are show-stoppers, and some of these features could be ported over. I'm leaving these notes because switching some of my projects over is made more difficult due to them.

@sentriz
Copy link

sentriz commented Nov 6, 2023

how might one try port their tests over to use rsc/script when its go.mod has go 1.22?

@twpayne
Copy link
Contributor Author

twpayne commented Nov 6, 2023

how might one try port their tests over to use rsc/script when its go.mod has go 1.22?

It's only a question of time until someone creates a fork without the go 1.22 requirement. Then you can use that.

However, I would not rush. Right now, everyone has tests passing with the framework that they're using.

@sentriz
Copy link

sentriz commented Nov 7, 2023

There appears to be no support for setting up programs in $PATH, which is really useful for developing tools that get executed indirectly

@mvdan it looks like Program can provide access to your host PATH
https://github.com/rsc/script/blob/v0.0.1/cmds.go#L802

@mvdan
Copy link
Collaborator

mvdan commented Nov 7, 2023

To be clear, that's not what I meant in my first point above. Some tools need to set up a PATH with their own binary to be tested, e.g. if the tool is called indirectly via exec go build -toolexec=mytool. That's what testscript supports with RunMain. You can think of script.Program as the exact opposite - it takes an existing program from the host's PATH and exposes it as a script command.

@sentriz
Copy link

sentriz commented Dec 6, 2023

it seems to also be missing the "stdin" command. unless i'm missing something, it seems you can't set the stdin of a proceeding command in the script

@twpayne
Copy link
Contributor Author

twpayne commented Mar 6, 2024

rsc has been silent so I asked on golang-nuts: https://groups.google.com/g/golang-nuts/c/leGd1uOXpIA

@twpayne
Copy link
Contributor Author

twpayne commented Mar 8, 2024

Russ responded:

https://groups.google.com/g/golang-nuts/c/leGd1uOXpIA/m/E7YYg6RbAQAJ
rsc/script#4

tl;dr rsc.io/script is a code dump to support a talk and is not accepting contributions to avoid divergence from cmd/go.

So, we should stick with testscript :)

@twpayne twpayne closed this as completed Mar 8, 2024
@mvdan
Copy link
Collaborator

mvdan commented Mar 8, 2024

Nice, thanks for following up, @twpayne.

I think it would still make sense to borrow good ideas and newer code from upstream's internal code, just like we have done in the past. But if they don't plan on doing a public or stable API anytime soon, then I don't think it makes sense for us to break our stable API for the foreseeable future, or to create a new package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants