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

all: test that all examples run on golang.org, i.e. the playground #19825

Open
mvdan opened this issue Apr 3, 2017 · 11 comments
Open

all: test that all examples run on golang.org, i.e. the playground #19825

mvdan opened this issue Apr 3, 2017 · 11 comments
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@mvdan
Copy link
Member

mvdan commented Apr 3, 2017

Right now, examples are only run via go test by the trybots. That checks whether or not they run as tests, but it doesn't check if they run in the playground.

For instance, see #19823. That example depends on its own test file, example_test.go. That will be available if one runs go test, but not if one presses "Run" in https://golang.org/pkg/go/parser/#example_ParseFile which will run it in the playground.

There should be a mechanism to ensure that all of the examples run on the golang.org docs too. In other words, that they pass when executed in the playground.

@mvdan
Copy link
Member Author

mvdan commented Apr 3, 2017

Perhaps there could be a separate trybot for this, similar to how there are race trybots. Not sure how the playground could be hooked up - not familiar enough with it.

CC @josharian

@josharian
Copy link
Contributor

We have a nacl builder, which should run the examples. That should cover this, although I'm obviously wrong about that somehow.

@mvdan
Copy link
Member Author

mvdan commented Apr 3, 2017

But I understand the nacl builder just does go test like the rest, correct?

@josharian
Copy link
Contributor

go test runs examples, though.

@mvdan
Copy link
Member Author

mvdan commented Apr 3, 2017

Yes, but if I understand correctly the nacl builder will still have example_test.go in the current directory when it runs that particular example via go test.

@josharian
Copy link
Contributor

I'm still obviously wrong somewhere, but each executed nacl process should not have access to the filesystem once started.

@mvdan
Copy link
Member Author

mvdan commented Apr 3, 2017

Ah, that makes sense and should do the trick. But it obviously doesn't :)

Perhaps we should add a test that only runs on nacl that checks that its own test file doesn't exist in disk, or that it cannot be opened?

@josharian
Copy link
Contributor

I'd say the first step is figuring out exactly why #19823 was passing on nacl.

@josharian josharian added help wanted Testing An issue that has been verified to require only test changes, not just a test failure. labels Apr 3, 2017
@josharian josharian added this to the Go1.9Maybe milestone Apr 3, 2017
@tzneal
Copy link
Member

tzneal commented Apr 10, 2017

As to why it passes, src/naclmake.bash packages up a fake file system for nacl tests according to misc/nacl/testzip.proto. On line 127 there it includes everything beneath src/go/parser which includes example_test.go.

Maybe the playground should use the same packaged filesystem?

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9Maybe Jul 20, 2017
@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 20, 2017
@ianlancetaylor ianlancetaylor modified the milestones: Go1.10, Unplanned Dec 6, 2017
@ysmolski
Copy link
Member

Do we know how many examples fail to run on playground?
@jimmyfrasche

@jimmyfrasche
Copy link
Member

This isn't up to date, but I did a run through earlier this year: #9679 (comment)

Since there have been improvements to how go/doc extracts examples, allowing more to be playable, some fixed examples, and more examples overall, someone's just going to need to run through each example and hit play (preferably a trybot).

I don't have a count but it's more than 1, so it's enough to warrant attention and action.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

7 participants