Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1413)

Issue 6501094: code review 6501094: testing: add Skip/Skipf (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 6 months ago by dfc
Modified:
11 years, 2 months ago
Reviewers:
CC:
adg, rsc, r, niemeyer, golang-dev, minux1
Visibility:
Public.

Description

testing: add Skip/Skipf This proposal adds two methods to *testing.T, Skip(string) and Skipf(format, args...). The intent is to replace the existing log and return idiom which currently has 97 cases in the standard library. A simple example of Skip would be: func TestSomethingLong(t *testing.T) { if testing.Short() { t.Skip("skipping test in short mode.") // not reached } ... time consuming work } Additionally tests can be skipped anywhere a *testing.T is present. An example adapted from the go.crypto/ssh/test package would be: // setup performs some before test action and returns a func() // which should be defered by the caller for cleanup. func setup(t *testing.T) func() { ... cmd := exec.Command("sshd", "-f", configfile, "-i") if err := cmd.Run(); err != nil { t.Skipf("could not execute mock ssh server: %v", err) } ... return func() { // stop subprocess and cleanup } } func TestDialMockServer(t *testing.T) { cleanup := setup(t) defer cleanup() ... } In verbose mode tests that are skipped are now reported as a SKIP, rather than PASS. Link to discussion: https://groups.google.com/d/topic/golang-nuts/BqorNARzt4U/discussion

Patch Set 1 #

Patch Set 2 : diff -r 91c1c2d6e2ff https://code.google.com/p/go #

Patch Set 3 : diff -r 9dacae220eca https://code.google.com/p/go #

Total comments: 1

Patch Set 4 : diff -r 9dacae220eca https://code.google.com/p/go #

Patch Set 5 : diff -r 9dacae220eca https://code.google.com/p/go #

Patch Set 6 : diff -r 073d912321cc https://code.google.com/p/go #

Patch Set 7 : diff -r 073d912321cc https://code.google.com/p/go #

Total comments: 2

Patch Set 8 : diff -r 9439d60e0f96 https://go.googlecode.com/hg/ #

Patch Set 9 : diff -r 9439d60e0f96 https://go.googlecode.com/hg/ #

Patch Set 10 : diff -r 9439d60e0f96 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+47 lines, -2 lines) Patch
M src/pkg/testing/testing.go View 1 2 3 4 5 6 7 4 chunks +47 lines, -2 lines 0 comments Download

Messages

Total messages: 12
dfc
Hello adg@golang.org, rsc@golang.org (cc: golang-dev@googlegroups.com, minux.ma@gmail.com), I'd like you to review this change to https://code.google.com/p/go
11 years, 2 months ago (2013-01-21 10:02:11 UTC) #1
r
If you're going to add this function, please use it in the existing tests with ...
11 years, 2 months ago (2013-01-21 14:03:51 UTC) #2
dfc
Will do, I will propose a follow up CL that uses it. I was following ...
11 years, 2 months ago (2013-01-21 18:53:05 UTC) #3
minux1
On Tue, Jan 22, 2013 at 2:52 AM, Dave Cheney <dave@cheney.net> wrote: > Will do, ...
11 years, 2 months ago (2013-01-21 18:57:55 UTC) #4
dfc
PTAL. I have updated the CL description and added an example of t.Skip() to the ...
11 years, 2 months ago (2013-01-22 01:06:46 UTC) #5
niemeyer
That's a great idea! ;)
11 years, 2 months ago (2013-01-22 01:55:14 UTC) #6
adg
https://codereview.appspot.com/6501094/diff/5001/src/pkg/testing/testing.go File src/pkg/testing/testing.go (right): https://codereview.appspot.com/6501094/diff/5001/src/pkg/testing/testing.go#newcode350 src/pkg/testing/testing.go:350: func (t *T) SkipNow() { document skipnow?
11 years, 2 months ago (2013-01-22 04:50:15 UTC) #7
dfc
> src/pkg/testing/testing.go:350: func (t *T) SkipNow() { > document skipnow? Done, and made unexported, 2 ...
11 years, 2 months ago (2013-01-22 04:54:07 UTC) #8
dfc
Hello adg@golang.org, rsc@golang.org, r@golang.org, n13m3y3r@gmail.com (cc: golang-dev@googlegroups.com, minux.ma@gmail.com), Please take another look.
11 years, 2 months ago (2013-01-22 04:57:54 UTC) #9
dfc
On 2013/01/22 04:54:07, dfc wrote: > > src/pkg/testing/testing.go:350: func (t *T) SkipNow() { > > ...
11 years, 2 months ago (2013-01-22 04:58:01 UTC) #10
rsc
LGTM https://codereview.appspot.com/6501094/diff/21001/src/pkg/testing/testing.go File src/pkg/testing/testing.go (right): https://codereview.appspot.com/6501094/diff/21001/src/pkg/testing/testing.go#newcode373 src/pkg/testing/testing.go:373: // Skipped returns whether the function was skipped. ...
11 years, 2 months ago (2013-01-22 22:22:19 UTC) #11
dfc
11 years, 2 months ago (2013-01-22 23:23:15 UTC) #12
*** Submitted as https://code.google.com/p/go/source/detail?r=0aa3f72c1c8b ***

testing: add Skip/Skipf

This proposal adds two methods to *testing.T, Skip(string) and Skipf(format,
args...). The intent is to replace the existing log and return idiom which
currently has 97 cases in the standard library. A simple example of Skip would
be:

func TestSomethingLong(t *testing.T) {
        if testing.Short() {
                t.Skip("skipping test in short mode.")
                // not reached
        }
        ... time consuming work
}

Additionally tests can be skipped anywhere a *testing.T is present. An example
adapted from the go.crypto/ssh/test package would be:

// setup performs some before test action and returns a func()
// which should be defered by the caller for cleanup.
func setup(t *testing.T) func() {
        ...
        cmd := exec.Command("sshd", "-f", configfile, "-i")
        if err := cmd.Run(); err != nil {
                t.Skipf("could not execute mock ssh server: %v", err)
        }
        ...
        return func() {
                // stop subprocess and cleanup
        }
}

func TestDialMockServer(t *testing.T) {
        cleanup := setup(t)
        defer cleanup()
        ...
}

In verbose mode tests that are skipped are now reported as a SKIP, rather than
PASS.

Link to discussion:
https://groups.google.com/d/topic/golang-nuts/BqorNARzt4U/discussion

R=adg, rsc, r, n13m3y3r
CC=golang-dev, minux.ma
https://codereview.appspot.com/6501094

https://codereview.appspot.com/6501094/diff/21001/src/pkg/testing/testing.go
File src/pkg/testing/testing.go (right):

https://codereview.appspot.com/6501094/diff/21001/src/pkg/testing/testing.go#...
src/pkg/testing/testing.go:373: // Skipped returns whether the function was
skipped.
On 2013/01/22 22:22:19, rsc wrote:
> s/returns/reports/

Done. I have also adjusted the docs for T.Failed().
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b