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: allow examples to return an error #21111

Open
rogpeppe opened this issue Jul 21, 2017 · 12 comments
Open

proposal: testing: allow examples to return an error #21111

rogpeppe opened this issue Jul 21, 2017 · 12 comments

Comments

@rogpeppe
Copy link
Contributor

@rogpeppe rogpeppe commented Jul 21, 2017

Currently, all example functions must return nothing. However, it is common
for examples to call things which might possibly return errors. Printing the
error, panicking, ignoring it or calling log.Fatal is not ideal for an example where
real code would almost always just return the error. It makes the example code
less representative of the actual code and more cluttered.

I propose that example functions should be allowed to return an error,
For runnable examples, tests would fail if the example
returns a non-nil error.

@rogpeppe rogpeppe added the Proposal label Jul 21, 2017
@rogpeppe rogpeppe changed the title testing: allow examples to return an error proposal: testing: allow examples to return an error Jul 21, 2017
@gopherbot gopherbot added this to the Proposal milestone Jul 21, 2017
@mvdan
Copy link
Member

@mvdan mvdan commented Jul 21, 2017

/cc @shurcooL @davecheney who I believe were discussing this on Twitter recently.

I believe they mentioned using something like if err != nil { /* handle err */ }.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 21, 2017

Right now a runnable example is simply one with an // Output: (or // Unordered output:) comment. So we could extend your suggestion by saying that any example that returns an error is runnable and will be run by go test. Returning a non-nil error would be reported as a test failure.

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Jul 21, 2017

@rogpeppe I think it would be helpful towards being able to evaluate what the end result could look like if you provided some examples of such examples. There may be subtle details that will become apparent once written down, instead of us having to use our imagination.

@mvdan I use a comment to indicate that error handling should take place in contexts such as READMEs, blog posts, snippets for reading, chat. E.g.:

// in a blog post

foo, err := Bar()
if err != nil {
    // Handle error.
}
foo.Baz()

// Output: "foo"

But in real executable Go examples, I handle errors, typically with panic:

// in example_test.go

func ExampleBar() {
    foo, err := Bar()
    if err != nil {
        panic(err)
    }
    fmt.Println(foo.Baz())

    // Output: "foo"
}

One of the concerns about having func ExampleBar() error signature is that it may require a superfluous return nil at the end:

func ExampleBar() error {
    foo, err := Bar()
    if err != nil {
        return err
    }
    fmt.Println(foo.Baz())
    return nil // this line isn't helping anyone but is neccessary

    // Output: "foo"
}

Will godoc display the final return nil? Will it detract from the example at hand? If godoc hides it, that's a special case (which needs to be implemented, documented, and adds additional mental complexity for all users of Go).

Another question is, if returning error is allowed, what about returning interface{}, error? So that I can write an even more representative example:

func ExampleBar() (*Baz, error) {
    foo, err := Bar()
    if err != nil {
        return nil, err
    }
    return foo.Baz(), nil
}

But then how does // Output: fit in there? That's one type of question this proposal opens up, and I think it's worth answering.

These are just some questions that come to mind.

Finally, I think a good exercise would be to find some typical examples from the Go standard library and see how they would be improved by this proposal. Would it be a significant improvement?

@rsc
Copy link
Contributor

@rsc rsc commented Jul 31, 2017

@rogpeppe, any thoughts on @shurcooL's latest reply?

@rsc
Copy link
Contributor

@rsc rsc commented Oct 9, 2017

ping @rogpeppe

@rogpeppe
Copy link
Contributor Author

@rogpeppe rogpeppe commented Oct 11, 2017

I think that removing the final return nil is fine, as we don't show the function declaration either - we're showing a snippet from a larger function. That said, I've just realised that golang.org (but not godoc.org) shows the example as a complete function with a main function. That leads to the question of how we should show that when the Example function returns an error.

Perhaps something like this:

package main
func example() error {
    // example code here
}
func main() {
    if err := example(); err != nil {
        log.Fatal(err)
    }
 }

When not displaying a full example (as godoc.org does) we could just show the example code without any trailing return nil.

I think that allowing a more arbitrary returned value is an interesting idea but brings its own set of issues and should be considered separately.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Mar 19, 2018

CC @dsnet

@rsc
Copy link
Contributor

@rsc rsc commented Apr 16, 2018

Printing the
error, panicking, ignoring it or calling log.Fatal is not ideal for an example where
real code would almost always just return the error. It makes the example code
less representative of the actual code and more cluttered.

I think this is a good point, but we are also talking about making error handling clearer in some future language change. It might be good to work that out before we decide what exactly to encourage in examples.

@rsc rsc added the Proposal-Hold label Apr 16, 2018
@rsc
Copy link
Contributor

@rsc rsc commented Apr 16, 2018

On hold for error handling.

@nigeltao
Copy link
Contributor

@nigeltao nigeltao commented Aug 29, 2018

The original proposal allows ExampleXxx functions to return error, instead of returning nothing. We could extend the proposal to also allow TestXxx and BenchmarkXxx functions to optionally return error. Having TestXxx return a non-nil err would implicitly call t.Error(err) on the t *testing.T and likewise for benchmarks.

Combined with the check and default-handler mechanisms suggested in the Error Handling Go 2 Draft Design, this could reduce some boilerplate.

To be clear, func TestXxx(t *testing.T) { etc } that returned no value would still be valid, and have unchanged semantics.

On the other hand, making it easier to reduce boilerplate for insufficiently useful test failures isn't necessary a good thing. Encouraging "return on first failure" also rubs against the table driven tests philosophy somewhat, where we teach that it's useful to distinguish e.g. "all test cases failed" from "every second test case failed" from "test cases failed when foobar is positive".

As per the OTOH, I'm not sure whether I actually like this idea, but I wanted to record it before I forget.

This is similar to issue #27328 but this one is for testing functions and that one is for main functions (which are somewhat similar to test examples).

@michael-schaller
Copy link
Contributor

@michael-schaller michael-schaller commented May 1, 2020

I'd like to present an example where it would IMHO make sense to let Examples return errors.

Let's take this function as an example:

// WriteTempFile creates a temporary file like ioutil.TempFile and writes the provided data to it.
// It also returns a cleanup function to defer the removal of the temporary file (including error handling in case the removal fails).
func WriteTempFile(dir string, pattern string, data []byte) (filename string, cleanup func(*error), err error)

The example I'd like to write would look like this:

func ExampleWriteTempFile() (err error) {
	filename, cleanup, err := WriteTempFile("", "example-*", []byte("example"))
	if err != nil {
		return err
	}
	defer cleanup(&err)

	// Use temporary file ...
	fmt.Printf("Temporary example file name: %s\n", filename)

	return nil
}

The example I currently have to write looks like this:

func ExampleWriteTempFile() {
	example := func() (err error) {
		filename, cleanup, err := WriteTempFile("", "example-*", []byte("example"))
		if err != nil {
			return err
		}
		defer cleanup(&err)

		// Use temporary file ...
		fmt.Printf("Temporary example file name: %s\n", filename)

		return nil
	}

	err := example()
	if err != nil {
		panic(err)
	}
	fmt.Println("no error")
	// Output: no error
}

You can play with this example on the playground.

@michael-schaller
Copy link
Contributor

@michael-schaller michael-schaller commented May 1, 2020

I think that Examples are a bit of a special case in unit tests as they should be as terse and expressive as possible so that users can easily grasp them. Hence it would IMHO be great if we would only discuss @rogpeppe's original proposal in this issue.

However I think @nigeltao's proposal has merit to consider this for Tests and Benchmarks as well, but IMHO Tests and Benchmarks are sufficiently different from Examples so that this should be discussed in a separate issue.

Additionally I think that the Error Handling Go 2 Draft Design is orthogonal to this issue as an Example might or might not use this kind of error handling depending if it makes an Example easier to grasp or not. Not to mention that this proposal is for Go 2 but fixing this could improve some Go 1 examples. That said it would be great if the Proposal-Hold label could be removed from this issue to resume the disucssion.

Last but not least I would even extend @rogpeppe's proposal to also allow Examples that return multiple values as long as the last return value is an error.

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

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.