https://codereview.appspot.com/6821100/diff/3002/src/pkg/net/protoconn_test.go File src/pkg/net/protoconn_test.go (right): https://codereview.appspot.com/6821100/diff/3002/src/pkg/net/protoconn_test.go#newcode26 src/pkg/net/protoconn_test.go:26: ln.SetDeadline(time.Now().Add(100 * time.Millisecond)) can you make this faster or ...
11 years, 4 months ago
(2012-11-09 15:54:54 UTC)
#3
On Sat, Nov 10, 2012 at 12:54 AM, <bradfitz@golang.org> wrote: > https://codereview.appspot.com/6821100/diff/3002/src/pkg/net/protoconn_test.go#newcode26 > src/pkg/net/protoconn_test.go:26: ln.SetDeadline(time.Now().Add(100 ...
11 years, 4 months ago
(2012-11-09 17:56:31 UTC)
#4
On Sat, Nov 10, 2012 at 12:54 AM, <bradfitz@golang.org> wrote:
>
https://codereview.appspot.com/6821100/diff/3002/src/pkg/net/protoconn_test.g...
> src/pkg/net/protoconn_test.go:26: ln.SetDeadline(time.Now().Add(100 *
> time.Millisecond))
> can you make this faster or make this test be skipped in -short mode?
sure, will do.
>
https://codereview.appspot.com/6821100/diff/3002/src/pkg/net/protoconn_test.g...
> src/pkg/net/protoconn_test.go:32: if f, err := ln.File(); err == nil {
> is this just clean-up, or do you care about the err != nil case here too
> for failing this test? it's not clear.
it's just kinda insurance. tests on the net package consume
sockets which consists of file descriptors and ephemeral ports.
currently a single net.test process takes all test cases. that
means that we can't rely on the platform's resource cleanup for
now.
>
https://codereview.appspot.com/6821100/diff/3002/src/pkg/net/protoconn_test.g...
> src/pkg/net/protoconn_test.go:64: c.WriteMsgUDP(wb, nil,
> c.LocalAddr().(*net.UDPAddr))
> I guess I don't understand the point of these tests, calling methods and
> ignoring their results.
>
> Just testing that it compiles so the API exists? Seems like goapi does
> that.
yes, and call them then see whether they crash.
will add simple use cases later, in another CL.
https://codereview.appspot.com/6821100/diff/8002/src/pkg/net/protoconn_test.go File src/pkg/net/protoconn_test.go (right): https://codereview.appspot.com/6821100/diff/8002/src/pkg/net/protoconn_test.go#newcode46 src/pkg/net/protoconn_test.go:46: case "plan9", "windows": are these TODOs or intentional? add ...
11 years, 4 months ago
(2012-11-10 02:41:45 UTC)
#6
thanks for the comment. PTAL. https://codereview.appspot.com/6821100/diff/8002/src/pkg/net/protoconn_test.go File src/pkg/net/protoconn_test.go (right): https://codereview.appspot.com/6821100/diff/8002/src/pkg/net/protoconn_test.go#newcode46 src/pkg/net/protoconn_test.go:46: case "plan9", "windows": On ...
11 years, 4 months ago
(2012-11-10 03:12:36 UTC)
#7
https://codereview.appspot.com/6821100/diff/11001/src/pkg/net/protoconn_test.go File src/pkg/net/protoconn_test.go (right): https://codereview.appspot.com/6821100/diff/11001/src/pkg/net/protoconn_test.go#newcode16 src/pkg/net/protoconn_test.go:16: func notImplementedYet() bool { all the call sites look ...
11 years, 4 months ago
(2012-11-10 04:03:21 UTC)
#8
https://codereview.appspot.com/6821100/diff/11001/src/pkg/net/protoconn_test.go
File src/pkg/net/protoconn_test.go (right):
https://codereview.appspot.com/6821100/diff/11001/src/pkg/net/protoconn_test....
src/pkg/net/protoconn_test.go:16: func notImplementedYet() bool {
all the call sites look like double negative not not implemented yet, which
reads like "if implemented yet", which is kinda weird.
what if this were instead:
var condFatalf = func() func (*testing.T, string, ...interface{}) {
// A few APIs are not implemented yet on both Plan 9 and Windows.
switch runtime.GOOS {
case "plan9", "windows":
return (*testing.T).Logf // or your own function, prepending the error
text with "Expected failure: "
}
return (*testing.T).Fatalf
}()
then
condFatalf(t, "net.TCPListener.File failed: %v", err)
condFatalf(t, "net.TCPListener.File failed: %v", err)
since it's not a wrapper function, you should still get the proper line numbers
in errors too from runtime.Caller.
On Sat, Nov 10, 2012 at 1:03 PM, <bradfitz@golang.org> wrote: > what if this were ...
11 years, 4 months ago
(2012-11-10 05:26:49 UTC)
#10
On Sat, Nov 10, 2012 at 1:03 PM, <bradfitz@golang.org> wrote:
> what if this were instead:
nice! thanks.
> since it's not a wrapper function, you should still get the proper line
> numbers in errors too from runtime.Caller.
i miss a sort of test mock which fills up platform, feature gaps.
*** Submitted as http://code.google.com/p/go/source/detail?r=90e59cec3396 *** net: add more tests for protocol specific methods R=golang-dev, bradfitz ...
11 years, 4 months ago
(2012-11-10 05:34:43 UTC)
#13
Issue 6821100: code review 6821100: net: add more tests for protocol specific methods
(Closed)
Created 11 years, 4 months ago by mikio
Modified 11 years, 4 months ago
Reviewers:
Base URL:
Comments: 8