FAQ
Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go


Description:
net: make unix connection tests more robust

Avoids unlink the underlying file before the socket close.

Please review this at https://codereview.appspot.com/7004044/

Affected files:
M src/pkg/net/conn_test.go
M src/pkg/net/packetconn_test.go


Index: src/pkg/net/conn_test.go
===================================================================
--- a/src/pkg/net/conn_test.go
+++ b/src/pkg/net/conn_test.go
@@ -17,8 +17,8 @@
addr string
}{
{"tcp", "127.0.0.1:0"},
- {"unix", "/tmp/gotest.net"},
- {"unixpacket", "/tmp/gotest.net"},
+ {"unix", "/tmp/gotest.net1"},
+ {"unixpacket", "/tmp/gotest.net2"},
}

func TestConnAndListener(t *testing.T) {
@@ -41,7 +41,13 @@
return
}
ln.Addr()
- defer ln.Close()
+ defer func(ln net.Listener, net, addr string) {
+ ln.Close()
+ switch net {
+ case "unix", "unixpacket":
+ os.Remove(addr)
+ }
+ }(ln, tt.net, tt.addr)

done := make(chan int)
go transponder(t, ln, done)
@@ -68,10 +74,6 @@
}

<-done
- switch tt.net {
- case "unix", "unixpacket":
- os.Remove(tt.addr)
- }
}
}

Index: src/pkg/net/packetconn_test.go
===================================================================
--- a/src/pkg/net/packetconn_test.go
+++ b/src/pkg/net/packetconn_test.go
@@ -24,6 +24,15 @@
}

func TestPacketConn(t *testing.T) {
+ closer := func(c net.PacketConn, net, addr1, addr2 string) {
+ c.Close()
+ switch net {
+ case "unixgram":
+ os.Remove(addr1)
+ os.Remove(addr2)
+ }
+ }
+
for _, tt := range packetConnTests {
var wb []byte
netstr := strings.Split(tt.net, ":")
@@ -39,7 +48,7 @@
continue
}
id := os.Getpid() & 0xffff
- wb = newICMPEchoRequest(id, 1, 128, []byte("IP PACKETCONN TEST "))
+ wb = newICMPEchoRequest(id, 1, 128, []byte("IP PACKETCONN TEST"))
case "unixgram":
switch runtime.GOOS {
case "plan9", "windows":
@@ -54,43 +63,43 @@

c1, err := net.ListenPacket(tt.net, tt.addr1)
if err != nil {
- t.Fatalf("net.ListenPacket failed: %v", err)
+ t.Errorf("net.ListenPacket failed: %v", err)
+ return
}
c1.LocalAddr()
c1.SetDeadline(time.Now().Add(100 * time.Millisecond))
c1.SetReadDeadline(time.Now().Add(100 * time.Millisecond))
c1.SetWriteDeadline(time.Now().Add(100 * time.Millisecond))
- defer c1.Close()
+ defer closer(c1, netstr[0], tt.addr1, tt.addr2)

c2, err := net.ListenPacket(tt.net, tt.addr2)
if err != nil {
- t.Fatalf("net.ListenPacket failed: %v", err)
+ t.Errorf("net.ListenPacket failed: %v", err)
+ return
}
c2.LocalAddr()
c2.SetDeadline(time.Now().Add(100 * time.Millisecond))
c2.SetReadDeadline(time.Now().Add(100 * time.Millisecond))
c2.SetWriteDeadline(time.Now().Add(100 * time.Millisecond))
- defer c2.Close()
+ defer closer(c2, netstr[0], tt.addr1, tt.addr2)

if _, err := c1.WriteTo(wb, c2.LocalAddr()); err != nil {
- t.Fatalf("net.PacketConn.WriteTo failed: %v", err)
+ t.Errorf("net.PacketConn.WriteTo failed: %v", err)
+ return
}
rb2 := make([]byte, 128)
if _, _, err := c2.ReadFrom(rb2); err != nil {
- t.Fatalf("net.PacketConn.ReadFrom failed: %v", err)
+ t.Errorf("net.PacketConn.ReadFrom failed: %v", err)
+ return
}
if _, err := c2.WriteTo(wb, c1.LocalAddr()); err != nil {
- t.Fatalf("net.PacketConn.WriteTo failed: %v", err)
+ t.Errorf("net.PacketConn.WriteTo failed: %v", err)
+ return
}
rb1 := make([]byte, 128)
if _, _, err := c1.ReadFrom(rb1); err != nil {
- t.Fatalf("net.PacketConn.ReadFrom failed: %v", err)
- }
-
- switch netstr[0] {
- case "unixgram":
- os.Remove(tt.addr1)
- os.Remove(tt.addr2)
+ t.Errorf("net.PacketConn.ReadFrom failed: %v", err)
+ return
}
}
}
@@ -118,7 +127,8 @@

c1, err := net.ListenPacket(tt.net, tt.addr1)
if err != nil {
- t.Fatalf("net.ListenPacket failed: %v", err)
+ t.Errorf("net.ListenPacket failed: %v", err)
+ return
}
c1.LocalAddr()
c1.SetDeadline(time.Now().Add(100 * time.Millisecond))
@@ -128,7 +138,8 @@

c2, err := net.Dial(tt.net, c1.LocalAddr().String())
if err != nil {
- t.Fatalf("net.Dial failed: %v", err)
+ t.Errorf("net.Dial failed: %v", err)
+ return
}
c2.LocalAddr()
c2.RemoteAddr()
@@ -138,11 +149,13 @@
defer c2.Close()

if _, err := c2.Write(wb); err != nil {
- t.Fatalf("net.Conn.Write failed: %v", err)
+ t.Errorf("net.Conn.Write failed: %v", err)
+ return
}
rb1 := make([]byte, 128)
if _, _, err := c1.ReadFrom(rb1); err != nil {
- t.Fatalf("net.PacetConn.ReadFrom failed: %v", err)
+ t.Errorf("net.PacetConn.ReadFrom failed: %v", err)
+ return
}
var dst net.Addr
if netstr[0] == "ip" {
@@ -151,11 +164,13 @@
dst = c2.LocalAddr()
}
if _, err := c1.WriteTo(wb, dst); err != nil {
- t.Fatalf("net.PacketConn.WriteTo failed: %v", err)
+ t.Errorf("net.PacketConn.WriteTo failed: %v", err)
+ return
}
rb2 := make([]byte, 128)
if _, err := c2.Read(rb2); err != nil {
- t.Fatalf("net.Conn.Read failed: %v", err)
+ t.Errorf("net.Conn.Read failed: %v", err)
+ return
}
}
}

Search Discussions

  • Dave at Dec 21, 2012 at 4:21 am
    Thank you, just one question.


    https://codereview.appspot.com/7004044/diff/3003/src/pkg/net/conn_test.go
    File src/pkg/net/conn_test.go (right):

    https://codereview.appspot.com/7004044/diff/3003/src/pkg/net/conn_test.go#newcode50
    src/pkg/net/conn_test.go:50: }(ln, tt.net, tt.addr)
    very nice.

    https://codereview.appspot.com/7004044/diff/3003/src/pkg/net/packetconn_test.go
    File src/pkg/net/packetconn_test.go (right):

    https://codereview.appspot.com/7004044/diff/3003/src/pkg/net/packetconn_test.go#newcode66
    src/pkg/net/packetconn_test.go:66: t.Errorf("net.ListenPacket failed:
    %v", err)
    Why have you converted all these to Errorfs ?

    https://codereview.appspot.com/7004044/
  • Mikio Hara at Dec 21, 2012 at 4:28 am

    On Fri, Dec 21, 2012 at 1:21 PM, wrote:

    Why have you converted all these to Errorfs ?
    to give a chance to run defer which contains socket/file closers.
  • Dave Cheney at Dec 21, 2012 at 4:32 am
    I don't think that is necessary, defers always run.

    lucky(~/t) % cat t_test.go
    package t

    import "testing"

    func TestErrorf(t *testing.T) {
    defer t.Logf("defer in TestErrorf")
    t.Errorf("bang")
    }

    func TestFatalf(t *testing.T) {
    defer t.Logf("defer in TestFatalf")
    t.Fatalf("boom")
    }
    lucky(~/t) % go test -v
    === RUN TestErrorf
    --- FAIL: TestErrorf (0.00 seconds)
    t_test.go:7: bang
    t_test.go:7: defer in TestErrorf
    === RUN TestFatalf
    --- FAIL: TestFatalf (0.00 seconds)
    t_test.go:12: boom
    panic.c:91: defer in TestFatalf
    FAIL
    exit status 1
    FAIL _/home/dfc/t 0.014s

    On Fri, Dec 21, 2012 at 3:28 PM, Mikio Hara wrote:
    On Fri, Dec 21, 2012 at 1:21 PM, wrote:

    Why have you converted all these to Errorfs ?
    to give a chance to run defer which contains socket/file closers.
  • Mikio Hara at Dec 21, 2012 at 4:53 am

    On Fri, Dec 21, 2012 at 1:32 PM, Dave Cheney wrote:

    I don't think that is necessary, defers always run.
    yup, but this test spawns a transponder goroutine and ah... right, thx. ;)
  • Mikioh Mikioh at Dec 21, 2012 at 4:54 am
    Hello golang-dev@googlegroups.com, dave@cheney.net (cc:
    golang-dev@googlegroups.com),

    Please take another look.


    https://codereview.appspot.com/7004044/
  • Dave at Dec 21, 2012 at 4:58 am
  • Mikioh Mikioh at Dec 21, 2012 at 5:19 am
    *** Submitted as
    https://code.google.com/p/go/source/detail?r=72648c5c21a1 ***

    net: make unix connection tests more robust

    Avoids unlink the underlying file before the socket close.

    R=golang-dev, dave
    CC=golang-dev
    https://codereview.appspot.com/7004044


    https://codereview.appspot.com/7004044/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedDec 21, '12 at 2:08a
activeDec 21, '12 at 5:19a
posts8
users2
websitegolang.org

2 users in discussion

Mikioh Mikioh: 5 posts Dave: 3 posts

People

Translate

site design / logo © 2022 Grokbase