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: testing: support running examples in parallel #35852

Closed
mvdan opened this issue Nov 26, 2019 · 9 comments
Closed

proposal: testing: support running examples in parallel #35852

mvdan opened this issue Nov 26, 2019 · 9 comments

Comments

@mvdan
Copy link
Member

@mvdan mvdan commented Nov 26, 2019

Summary

Examples that run with go test always do so sequentially with all other examples and tests. I think this doesn't scale well at all with the size of packages or amount of examples, and encourages developers to only write a few simple examples per package.

I propose two solutions below; I lean towards the second one, which is more powerful.

Description

We can have examples which show up in the godoc but aren't runnable, and runnable examples which are run as part of go test. This is enough for most use cases.

However, take the case where one has more than a handful of examples in a single package, and they each take a non-trivial amount of time to run, such as 200ms. Since they cannot run in parallel with themselves or any other test within the same package, go test can take multiple seconds than it really needs to.

The root of the problem is that there's no way to do the equivalent of a test's t.Parallel() for a test.

Below are some workarounds I considered:

  • Rewriting some of the examples as tests. Not good, because they are lost in the godoc, and hidden from the users.
  • Splitting the examples in multiple packages. This makes go test ./... faster, as there's built-in parallelism between packages being tested. However, this splits up godoc pages, and it's not unreasonable to have more than a handful of examples in a single package.
  • Make some of the examples non-runnable. Perhaps the best option today, but still not great. A runnable example is always verified by go test, and it's easier for the user to see what it does and how to run it.

I propose two solutions to this.

Solution one: a variant of // Output:

A different special comment that marks the example as parallel. For example:

func ExampleFoo() {
    ...

    // Output (parallel):
    // ...
}

The syntax here is up to bikeshedding, but the idea behind the parentheses are to allow more "modes" in the future, to mirror more testing.T methods like Parallel().

Solution two: add testing.E

To mirror testing.T, offering a subset of the methods such as Parallel(). An example could optionally take it as a parameter:

func ExampleFoo(e *testing.E) {
    e.Parallel()
    ...

    // Output:
    // ...
}

If we don't want e.Parallel() to be part of the example code, we could add another special comment to specify when the example code actually starts:

func ExampleFoo(e *testing.E) {
    e.Parallel()

    // Example:
    ...

    // Output:
    // ...
}

This change is more invasive, but it's also more consistent with tests, and allows to more easily extend examples in the future. For example, we could also use this solution to solve #31310 via a testing.E.Skipf method.

We could also even fix other examples issues like #21111; that one could be done via e.Error or e.Fatal, which I'd say is significantly better than log.Fatal or panic.

testing.E could implement testing.TB if we want, but I don't think it's necessary.

I prefer this second solution, because it's more powerful, and doesn't add more syntax to the special // Output: comment.

@mvdan mvdan added the Proposal label Nov 26, 2019
@mvdan mvdan added this to the Proposal milestone Nov 26, 2019
@bcmills
Copy link
Member

@bcmills bcmills commented Nov 26, 2019

One interesting possibility for the // Output: variant is to also allow the output to be in a nondeterministic order. For example, you could consider something like

// Output (unordered):

This crops up in examples for concurrency libraries, such as in golang.org/x/sync/errgroup.ExampleGroup_pipeline. That example should be runnable, but it currently is not because the output is emitted in nondeterministic order.

Loading

@mvdan
Copy link
Member Author

@mvdan mvdan commented Nov 26, 2019

That already exists as // Unordered output:, I think. That's another reason why I think we shouldn't extend the current comment syntax; // Unordered output (parallel): seems a bit heavy to me.

Loading

@rsc
Copy link
Contributor

@rsc rsc commented Nov 27, 2019

The proposal here focuses on a way to signal that an example should be run in parallel.
This misses the larger problem that examples cannot run in parallel: they write to os.Stdout.
Each example's output needs to be captured separately.
There can only be one os.Stdout at a time, therefore only one example running at a time.
See testing/run_example.go's runExample function.
This is pretty fundamental to what an example is: as soon as you add a testing.E
and require prints to some other explicit place,
the examples stop being copy-and-paste-able code.

I don't see an obvious path forward here other than write examples that run quicker.

Loading

@mvdan
Copy link
Member Author

@mvdan mvdan commented Nov 27, 2019

You raise a good point about os.Stdout; I hadn't realised it was being modified like that. I definitely don't want to force rewriting of existing example code.

At the same time, I find "just write faster examples" to not be a satisfactory solution here. It feels akin to "just write faster tests" instead of writing parallel tests. For example, I have a package which drives Chrome browsers; examples are self-contained, so they must start a new Chrome process and talk to it to perform some example actions. There is absolutely no way I can make those examples run faster. All the other alternatives I've listed before are similarly unsatisfactory.

Are we sure there are no other ways to run current examples in parallel? Since they modify global state by design, how about running them as separate processes, kind of like how cmd/go's test scripts exec their own test binary to run programs?

Loading

@rsc rsc changed the title Proposal: testing: support running examples in parallel proposal: testing: support running examples in parallel Dec 4, 2019
@rsc rsc added this to Incoming in Proposals Dec 4, 2019
@bcmills
Copy link
Member

@bcmills bcmills commented Dec 5, 2019

There can only be one os.Stdout at a time, therefore only one example running at a time.

The mapping of the os and fmt identifiers depends upon import statements that are necessarily outside of the Example function itself.

It's true that if the code under test prints to os.Stdout or os.Stderr itself, we cannot test that code in parallel, but in many examples the only output is generated within the Example function itself (typically as explicit calls to fmt.Print*).

So one option to preserve copy-and-pasteability might be to scope the os and/or fmt identifiers to the Example function itself:

func ExampleFoo(e *testing.E) {
	fmt := e.MockFmt()
	e.Parallel()

	// Example:
	fmt.Println(42)

	// Output:
	// 42
}

Loading

@mvdan
Copy link
Member Author

@mvdan mvdan commented Dec 20, 2019

I still think that separate processes is an option, but I'd be fine with @bcmills' solution as well. Existing examples would behave the same, while those wanting parallelism would need to add a couple of lines with *testing.E.

Loading

@rsc
Copy link
Contributor

@rsc rsc commented Jan 8, 2020

It's also not only os.Stdout and os.Stderr. Examples are just meant to be run one at a time. I admit the problem here but it was just a failure of vision in the design. I don't see any way to fix it, and I'm not convinced it's particularly important.

The difference between "write faster examples" and "write faster tests" is that examples are meant to be trivial illustrations, while tests are not.

This seems like a (reluctant) likely decline.

Leaving open for a week for final comments.

Loading

@rsc rsc moved this from Incoming to Likely Decline in Proposals Jan 8, 2020
@mvdan
Copy link
Member Author

@mvdan mvdan commented Jan 9, 2020

It's also not only os.Stdout and os.Stderr. Examples are just meant to be run one at a time. [...] I don't see any way to fix it, and I'm not convinced it's particularly important.

If you mean owning the entire process and global state, what I briefly mentioned in #35852 (comment) was to run parallel examples by exec-ing the current test binary, in a way that it only runs the example, capturing its output. For those tests that do more work and need to be parallel, the overhead wouldn't matter. Existing examples wouldn't be affected.

That seems like a fine solution to me. I don't think "examples are meant to be trivial" is a good outcome here.

If we're in a world where examples are only for trivial stuff, where am I supposed to put non-trivial runnable examples? As I explained in the original post, all alternatives are terrible in their own way.
Our current alternative for https://godoc.org/github.com/chromedp/chromedp is https://github.com/chromedp/examples, which is hard to find, breaks easily, requires a separate module, etc etc.

Loading

@rsc
Copy link
Contributor

@rsc rsc commented Jan 15, 2020

If there is startup code that runs for a while, then re-execing the binary will duplicate all of that by introducing more startup time. I don't think that's a viable solution here. There's just not a path forward here short of completely redefining what examples are. I'm sorry about that, but pressure to keep examples short is not all bad.

No change in consensus, so declining.

Loading

@rsc rsc closed this Jan 15, 2020
@rsc rsc moved this from Likely Decline to Declined in Proposals Jan 15, 2020
@golang golang locked and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Proposals
Declined
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants