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

testing: no specific message if a benchmark is skipped #14799

Closed
pierrre opened this issue Mar 13, 2016 · 5 comments

Comments

Projects
None yet
5 participants
@pierrre
Copy link

commented Mar 13, 2016

  1. What version of Go are you using (go version)?
    1.6 linux/amd64
  2. What did you do?
    Skip a benchmark *testing.B
  3. What did you expect to see?
    It should display a message, like *testing.T
  4. What did you see instead?
    This is from my project.
    It skips tests and benchmark if there is no Redis server listening on the default port.
    As you can see, there are "SKIP" messages if a test is skipped, but only the error message if a benchmark is skipped.
➜  ~ go test -v -bench=. github.com/pierrre/imageserver/cache/redis
=== RUN   TestGetSet
--- SKIP: TestGetSet (0.00s)
    redis_test.go:115: dial tcp [::1]:6379: getsockopt: connection refused
=== RUN   TestGetMiss
--- SKIP: TestGetMiss (0.00s)
    redis_test.go:115: dial tcp [::1]:6379: getsockopt: connection refused
=== RUN   TestGetErrorAddress
--- PASS: TestGetErrorAddress (0.00s)
=== RUN   TestSetErrorAddress
--- PASS: TestSetErrorAddress (0.00s)
=== RUN   TestGetErrorUnmarshal
--- SKIP: TestGetErrorUnmarshal (0.00s)
    redis_test.go:115: dial tcp [::1]:6379: getsockopt: connection refused
=== RUN   TestSetErrorMarshal
--- SKIP: TestSetErrorMarshal (0.00s)
    redis_test.go:115: dial tcp [::1]:6379: getsockopt: connection refused
PASS
BenchmarkGetSizeSmall-8                0             0 ns/op
--- BENCH: BenchmarkGetSizeSmall-8
    redis_test.go:115: dial tcp [::1]:6379: getsockopt: connection refused
BenchmarkGetSizeMedium-8               0             0 ns/op
--- BENCH: BenchmarkGetSizeMedium-8
    redis_test.go:115: dial tcp [::1]:6379: getsockopt: connection refused
BenchmarkGetSizeLarge-8                0             0 ns/op
--- BENCH: BenchmarkGetSizeLarge-8
    redis_test.go:115: dial tcp [::1]:6379: getsockopt: connection refused
BenchmarkGetSizeHuge-8                 0             0 ns/op
--- BENCH: BenchmarkGetSizeHuge-8
    redis_test.go:115: dial tcp [::1]:6379: getsockopt: connection refused
BenchmarkGetParallelism1-8             0             0 ns/op
--- BENCH: BenchmarkGetParallelism1-8
    redis_test.go:115: dial tcp [::1]:6379: getsockopt: connection refused
BenchmarkGetParallelism2-8             0             0 ns/op
--- BENCH: BenchmarkGetParallelism2-8
    redis_test.go:115: dial tcp [::1]:6379: getsockopt: connection refused
BenchmarkGetParallelism4-8             0             0 ns/op
--- BENCH: BenchmarkGetParallelism4-8
    redis_test.go:115: dial tcp [::1]:6379: getsockopt: connection refused
BenchmarkGetParallelism8-8             0             0 ns/op
--- BENCH: BenchmarkGetParallelism8-8
    redis_test.go:115: dial tcp [::1]:6379: getsockopt: connection refused
BenchmarkGetParallelism16-8            0             0 ns/op
--- BENCH: BenchmarkGetParallelism16-8
    redis_test.go:115: dial tcp [::1]:6379: getsockopt: connection refused
BenchmarkGetParallelism32-8            0             0 ns/op
--- BENCH: BenchmarkGetParallelism32-8
    redis_test.go:115: dial tcp [::1]:6379: getsockopt: connection refused
BenchmarkGetParallelism64-8            0             0 ns/op
--- BENCH: BenchmarkGetParallelism64-8
    redis_test.go:115: dial tcp [::1]:6379: getsockopt: connection refused
BenchmarkGetParallelism128-8           0             0 ns/op
--- BENCH: BenchmarkGetParallelism128-8
    redis_test.go:115: dial tcp [::1]:6379: getsockopt: connection refused
ok      github.com/pierrre/imageserver/cache/redis  0.014s

@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Mar 14, 2016

@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 3, 2016

Simplified repro:

package benchskip

import "testing"

func TestSkip(t *testing.T) {
        t.Skip("skipping")
}

func BenchmarkSkip(b *testing.B) {
        b.Skip("skipping")
}

And running it:

$ go test -v -run=. -bench=.
=== RUN   TestSkip
--- SKIP: TestSkip (0.00s)
        benchskip_test.go:6: skipping
BenchmarkSkip-2                0                 0 ns/op
--- BENCH: BenchmarkSkip-2
        benchskip_test.go:10: skipping
        benchskip_test.go:10: skipping
PASS
ok      benchskip       0.003s

Three bugs:

  1. no SKIP line for benchmarks
  2. double "skipping" output for benchmarks
  3. benchmark times are output anyway, despite the skip.

@mpvl, you touched all this last. Can you take this?

@bradfitz bradfitz modified the milestones: Go1.7, Unplanned Apr 3, 2016

@mpvl

This comment has been minimized.

Copy link
Member

commented Apr 3, 2016

Yes, will do

On Sunday, April 3, 2016, Brad Fitzpatrick notifications@github.com wrote:

Simplified repro:

package benchskip
import "testing"
func TestSkip(t *testing.T) {
t.Skip("skipping")
}
func BenchmarkSkip(b *testing.B) {
b.Skip("skipping")
}

And running it:

$ go test -v -run=. -bench=.
=== RUN TestSkip
--- SKIP: TestSkip (0.00s)
benchskip_test.go:6: skipping
BenchmarkSkip-2 0 0 ns/op
--- BENCH: BenchmarkSkip-2
benchskip_test.go:10: skipping
benchskip_test.go:10: skipping
PASS
ok benchskip 0.003s

Three bugs:

  1. no SKIP line for benchmarks
  2. double "skipping" output for benchmarks
  3. benchmark times are output anyway, despite the skip.

@mpvl https://github.com/mpvl, you touched all this last. Can you take
this?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#14799 (comment)

@mpvl

This comment has been minimized.

Copy link
Member

commented Apr 4, 2016

The only recent bug of these three is the the doubling of output.
The request to display a SKIP-ed benchmark seems reasonable.
The benchmark times always being output was an artifact of the old implementation that was adopted by the recent implementation of subbenchmarks. Now we have a separate probing stage, though, it is easy to omit this line, at least for the common case where a failure is always present in the first iteration.

An upcoming CL addresses all of these issues.

@gopherbot

This comment has been minimized.

Copy link

commented Apr 4, 2016

CL https://golang.org/cl/21504 mentions this issue.

@gopherbot gopherbot closed this in 7e0d660 Apr 5, 2016

@pierrre

This comment has been minimized.

Copy link
Author

commented Apr 5, 2016

Thank you!

@golang golang locked and limited conversation to collaborators Apr 5, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.