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

proposal: cmd/go: add a test flag to update runnable example output comments #51254

Open
mvdan opened this issue Feb 18, 2022 · 4 comments
Open
Milestone

Comments

@mvdan
Copy link
Member

mvdan commented Feb 18, 2022

We have support for examples which can be run as part of go test, as documented in https://pkg.go.dev/testing#hdr-Examples.

Right now, if I change the example code in any way that changes its output, such as by tweaking the package API implementations, I'll get a failure when running go test again. With #41980, that failure will show me a diff, which is pretty neat.

However, if I do intend to change the output of the example for good, I'll need to manually update the // Output: comment lines in the example code, such as by copying the output text from the output of go test, pasting it in the editor, commenting out those lines, and placing them where the old ones are.

It would be nice if we instead had a flag, like go test -updateoutput but with a better name, so that could happen in-place automatically. It could also work with unordered output lines, which could keep the source code deterministic by e.g. sorting the lines.

The reason it would be a flag is because the existing default behavior should be kept. By default with go test, a runnable example whose output doesn't match should result in a failure.

It should be noted that @cznic has implemented an external version of this in https://pkg.go.dev/modernc.org/fe. It can certainly work outside of cmd/go, though I'd argue that it would be nice to have it built-in, akin to how we have other cmd/go commands to edit source in-place such as go fix or go mod edit.

One question might be: how often does this come up? I personally run into this scenario relatively often, as I try to design end-user Go APIs by first writing one or two examples, often in runnable form. As I iterate on implementing the logic of the APIs, I want to verify that their output becomes what I expect, akin to re-running a test until it succeeds.

This may not need to be a proposal, but since the suggestion is to add a new go test flag, I'm erring on the side of caution - it may be a controversial feature request :)

cc @bcmills @matloob (Bryan and I briefly discussed this idea on Slack back in September 2021)

@seankhliao
Copy link
Member

seankhliao commented Feb 18, 2022

or maybe under go fix since having go test change the source code seems a bit surprising.

@mvdan
Copy link
Member Author

mvdan commented Feb 18, 2022

That would require go fix to run runnable tests though, which might be more awkward. I think go test having an opt-in behavior to edit the source code is reasonable, akin to how go build -mod=mod may edit your go.mod.

@earthboundkid
Copy link
Contributor

I have run into cases before where it was hard to get the output comment correct because there was some trivial whitespace that didn’t affect overall correctness but was hard to get right and copypaste through the terminal was slightly off (I forget the exact details but it was like some tabs mixed with spaces which weren’t copying correctly or something). Having this feature would have been very helpful.

@rsc rsc moved this to Incoming in Proposals Aug 10, 2022
@rsc rsc added this to Proposals Aug 10, 2022
@smikulcik
Copy link

This example pattern is very similar to the NodeJS library, Jest, with its "Snapshot" pattern.

https://jestjs.io/docs/snapshot-testing

run tests to validate that the examples match:

jest

update the examples on disk:

jest --updateSnapshot

I'd be super excited to see this in Go!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Incoming
Development

No branches or pull requests

5 participants