Merge pull request #1119 from tbarker25/process-fixes-1

Fixes for Test_Connections in process_linux.go

process/process_linux.go

@@ -351,7 +351,7 @@ func (p *Process) PageFaultsWithContext(ctx context.Context) (*PageFaultsStat, e
 func (p *Process) ChildrenWithContext(ctx context.Context) ([]*Process, error) {
 	pids, err := common.CallPgrepWithContext(ctx, invoke, p.Pid)
 	if err != nil {
-		if pids == nil || len(pids) == 0 {
+		if len(pids) == 0 {
 			return nil, ErrorNoChildren
 		}
 		return nil, err
@@ -705,10 +705,7 @@ func (p *Process) fillFromCmdlineWithContext(ctx context.Context) (string, error
 		return "", err
 	}
 	ret := strings.FieldsFunc(string(cmdline), func(r rune) bool {
-		if r == '\u0000' {
+		return r == '\u0000'
-			return true
-		}
-		return false
 	})
 
 	return strings.Join(ret, " "), nil

process/process_test.go

@@ -418,14 +418,14 @@ func Test_Process_Exe(t *testing.T) {
 
 func Test_Process_CpuPercent(t *testing.T) {
 	p := testGetProcess()
-	percent, err := p.Percent(0)
+	_, err := p.Percent(0)
 	skipIfNotImplementedErr(t, err)
 	if err != nil {
 		t.Errorf("error %v", err)
 	}
 	duration := time.Duration(1000) * time.Microsecond
 	time.Sleep(duration)
-	percent, err = p.Percent(0)
+	percent, err := p.Percent(0)
 	if err != nil {
 		t.Errorf("error %v", err)
 	}
@@ -498,55 +498,84 @@ func Test_Parent(t *testing.T) {
 
 func Test_Connections(t *testing.T) {
 	p := testGetProcess()
-	ch0 := make(chan string)
+
-	ch1 := make(chan string)
+	addr, err := net.ResolveTCPAddr("tcp", "localhost:0") // dynamically get a random open port from OS
+	if err != nil {
+		t.Fatalf("unable to resolve localhost: %v", err)
+	}
+	l, err := net.ListenTCP(addr.Network(), addr)
+	if err != nil {
+		t.Fatalf("unable to listen on %v: %v", addr, err)
+	}
+	defer l.Close()
+
+	tcpServerAddr := l.Addr().String()
+	tcpServerAddrIP := strings.Split(tcpServerAddr, ":")[0]
+	tcpServerAddrPort, err := strconv.ParseUint(strings.Split(tcpServerAddr, ":")[1], 10, 32)
+	if err != nil {
+		t.Fatalf("unable to parse tcpServerAddr port: %v", err)
+	}
+
+	serverEstablished := make(chan struct{})
 	go func() { // TCP listening goroutine
-		addr, err := net.ResolveTCPAddr("tcp", "localhost:0") // dynamically get a random open port from OS
+		conn, err := l.Accept()
 		if err != nil {
-			t.Skip("unable to resolve localhost:", err)
+			panic(err)
 		}
-		l, err := net.ListenTCP(addr.Network(), addr)
+		defer conn.Close()
+
+		serverEstablished <- struct{}{}
+		_, err = ioutil.ReadAll(conn)
 		if err != nil {
-			t.Skip(fmt.Sprintf("unable to listen on %v: %v", addr, err))
+			panic(err)
-		}
-		defer l.Close()
-		ch0 <- l.Addr().String()
-		for {
-			conn, err := l.Accept()
-			if err != nil {
-				t.Skip("unable to accept connection:", err)
-			}
-			ch1 <- l.Addr().String()
-			defer conn.Close()
 		}
 	}()
-	go func() { // TCP client goroutine
-		tcpServerAddr := <-ch0
-		net.Dial("tcp", tcpServerAddr)
-	}()
 
-	tcpServerAddr := <-ch1
+	conn, err := net.Dial("tcp", tcpServerAddr)
-	tcpServerAddrIP := strings.Split(tcpServerAddr, ":")[0]
-	tcpServerAddrPort, err := strconv.ParseUint(strings.Split(tcpServerAddr, ":")[1], 10, 32)
 	if err != nil {
-		t.Errorf("unable to parse tcpServerAddr port: %v", err)
+		t.Fatalf("unable to dial %v: %v", tcpServerAddr, err)
 	}
+	defer conn.Close()
+
+	// Rarely the call to net.Dial returns before the server connection is
+	// established. Wait so that the test doesn't fail.
+	<-serverEstablished
+
 	c, err := p.Connections()
 	skipIfNotImplementedErr(t, err)
 	if err != nil {
-		t.Errorf("error %v", err)
+		t.Fatalf("error %v", err)
 	}
 	if len(c) == 0 {
-		t.Errorf("no connections found")
+		t.Fatal("no connections found")
 	}
-	found := 0
+
+	serverConnections := 0
+	for _, connection := range c {
+		if connection.Laddr.IP == tcpServerAddrIP && connection.Laddr.Port == uint32(tcpServerAddrPort) && connection.Raddr.Port != 0 {
+			if connection.Status != "ESTABLISHED" {
+				t.Fatalf("expected server connection to be ESTABLISHED, have %+v", connection)
+			}
+			serverConnections++
+		}
+	}
+
+	clientConnections := 0
 	for _, connection := range c {
-		if connection.Status == "ESTABLISHED" && (connection.Laddr.IP == tcpServerAddrIP && connection.Laddr.Port == uint32(tcpServerAddrPort)) || (connection.Raddr.IP == tcpServerAddrIP && connection.Raddr.Port == uint32(tcpServerAddrPort)) {
+		if connection.Raddr.IP == tcpServerAddrIP && connection.Raddr.Port == uint32(tcpServerAddrPort) {
-			found++
+			if connection.Status != "ESTABLISHED" {
+				t.Fatalf("expected client connection to be ESTABLISHED, have %+v", connection)
+			}
+			clientConnections++
 		}
 	}
-	if found != 2 { // two established connections, one for the server, the other for the client
+
-		t.Errorf(fmt.Sprintf("wrong connections: %+v", c))
+	if serverConnections != 1 { // two established connections, one for the server, the other for the client
+		t.Fatalf("expected 1 server connection, have %d.\nDetails: %+v", serverConnections, c)
+	}
+
+	if clientConnections != 1 { // two established connections, one for the server, the other for the client
+		t.Fatalf("expected 1 server connection, have %d.\nDetails: %+v", clientConnections, c)
 	}
 }
 
@@ -747,34 +776,38 @@ func Test_Process_Environ(t *testing.T) {
 
 func Test_AllProcesses_cmdLine(t *testing.T) {
 	procs, err := Processes()
-	if err == nil {
+	skipIfNotImplementedErr(t, err)
-		for _, proc := range procs {
+	if err != nil {
-			var exeName string
+		t.Fatalf("getting processes error %v", err)
-			var cmdLine string
+	}
-
+	for _, proc := range procs {
-			exeName, _ = proc.Exe()
+		var exeName string
-			cmdLine, err = proc.Cmdline()
+		var cmdLine string
-			if err != nil {
-				cmdLine = "Error: " + err.Error()
-			}
 
-			t.Logf("Process #%v: Name: %v / CmdLine: %v\n", proc.Pid, exeName, cmdLine)
+		exeName, _ = proc.Exe()
+		cmdLine, err = proc.Cmdline()
+		if err != nil {
+			cmdLine = "Error: " + err.Error()
 		}
+
+		t.Logf("Process #%v: Name: %v / CmdLine: %v\n", proc.Pid, exeName, cmdLine)
 	}
 }
 
 func Test_AllProcesses_environ(t *testing.T) {
 	procs, err := Processes()
-	if err == nil {
+	skipIfNotImplementedErr(t, err)
-		for _, proc := range procs {
+	if err != nil {
-			exeName, _ := proc.Exe()
+		t.Fatalf("getting processes error %v", err)
-			environ, err := proc.Environ()
+	}
-			if err != nil {
+	for _, proc := range procs {
-				environ = []string{"Error: " + err.Error() }
+		exeName, _ := proc.Exe()
-			}
+		environ, err := proc.Environ()
-
+		if err != nil {
-			t.Logf("Process #%v: Name: %v / Environment Variables: %v\n", proc.Pid, exeName, environ)
+			environ = []string{"Error: " + err.Error()}
 		}
+
+		t.Logf("Process #%v: Name: %v / Environment Variables: %v\n", proc.Pid, exeName, environ)
 	}
 }
 

v3/process/process_linux.go

@@ -351,7 +351,7 @@ func (p *Process) PageFaultsWithContext(ctx context.Context) (*PageFaultsStat, e
 func (p *Process) ChildrenWithContext(ctx context.Context) ([]*Process, error) {
 	pids, err := common.CallPgrepWithContext(ctx, invoke, p.Pid)
 	if err != nil {
-		if pids == nil || len(pids) == 0 {
+		if len(pids) == 0 {
 			return nil, ErrorNoChildren
 		}
 		return nil, err
@@ -678,10 +678,7 @@ func (p *Process) fillFromCmdlineWithContext(ctx context.Context) (string, error
 		return "", err
 	}
 	ret := strings.FieldsFunc(string(cmdline), func(r rune) bool {
-		if r == '\u0000' {
+		return r == '\u0000'
-			return true
-		}
-		return false
 	})
 
 	return strings.Join(ret, " "), nil

v3/process/process_test.go

@@ -420,14 +420,14 @@ func Test_Process_Exe(t *testing.T) {
 
 func Test_Process_CpuPercent(t *testing.T) {
 	p := testGetProcess()
-	percent, err := p.Percent(0)
+	_, err := p.Percent(0)
 	skipIfNotImplementedErr(t, err)
 	if err != nil {
 		t.Errorf("error %v", err)
 	}
 	duration := time.Duration(1000) * time.Microsecond
 	time.Sleep(duration)
-	percent, err = p.Percent(0)
+	percent, err := p.Percent(0)
 	if err != nil {
 		t.Errorf("error %v", err)
 	}
@@ -500,55 +500,84 @@ func Test_Parent(t *testing.T) {
 
 func Test_Connections(t *testing.T) {
 	p := testGetProcess()
-	ch0 := make(chan string)
+
-	ch1 := make(chan string)
+	addr, err := net.ResolveTCPAddr("tcp", "localhost:0") // dynamically get a random open port from OS
+	if err != nil {
+		t.Fatalf("unable to resolve localhost: %v", err)
+	}
+	l, err := net.ListenTCP(addr.Network(), addr)
+	if err != nil {
+		t.Fatalf("unable to listen on %v: %v", addr, err)
+	}
+	defer l.Close()
+
+	tcpServerAddr := l.Addr().String()
+	tcpServerAddrIP := strings.Split(tcpServerAddr, ":")[0]
+	tcpServerAddrPort, err := strconv.ParseUint(strings.Split(tcpServerAddr, ":")[1], 10, 32)
+	if err != nil {
+		t.Fatalf("unable to parse tcpServerAddr port: %v", err)
+	}
+
+	serverEstablished := make(chan struct{})
 	go func() { // TCP listening goroutine
-		addr, err := net.ResolveTCPAddr("tcp", "localhost:0") // dynamically get a random open port from OS
+		conn, err := l.Accept()
 		if err != nil {
-			t.Skip("unable to resolve localhost:", err)
+			panic(err)
 		}
-		l, err := net.ListenTCP(addr.Network(), addr)
+		defer conn.Close()
+
+		serverEstablished <- struct{}{}
+		_, err = ioutil.ReadAll(conn)
 		if err != nil {
-			t.Skip(fmt.Sprintf("unable to listen on %v: %v", addr, err))
+			panic(err)
-		}
-		defer l.Close()
-		ch0 <- l.Addr().String()
-		for {
-			conn, err := l.Accept()
-			if err != nil {
-				t.Skip("unable to accept connection:", err)
-			}
-			ch1 <- l.Addr().String()
-			defer conn.Close()
 		}
 	}()
-	go func() { // TCP client goroutine
-		tcpServerAddr := <-ch0
-		net.Dial("tcp", tcpServerAddr)
-	}()
 
-	tcpServerAddr := <-ch1
+	conn, err := net.Dial("tcp", tcpServerAddr)
-	tcpServerAddrIP := strings.Split(tcpServerAddr, ":")[0]
-	tcpServerAddrPort, err := strconv.ParseUint(strings.Split(tcpServerAddr, ":")[1], 10, 32)
 	if err != nil {
-		t.Errorf("unable to parse tcpServerAddr port: %v", err)
+		t.Fatalf("unable to dial %v: %v", tcpServerAddr, err)
 	}
+	defer conn.Close()
+
+	// Rarely the call to net.Dial returns before the server connection is
+	// established. Wait so that the test doesn't fail.
+	<-serverEstablished
+
 	c, err := p.Connections()
 	skipIfNotImplementedErr(t, err)
 	if err != nil {
-		t.Errorf("error %v", err)
+		t.Fatalf("error %v", err)
 	}
 	if len(c) == 0 {
-		t.Errorf("no connections found")
+		t.Fatal("no connections found")
 	}
-	found := 0
+
+	serverConnections := 0
+	for _, connection := range c {
+		if connection.Laddr.IP == tcpServerAddrIP && connection.Laddr.Port == uint32(tcpServerAddrPort) && connection.Raddr.Port != 0 {
+			if connection.Status != "ESTABLISHED" {
+				t.Fatalf("expected server connection to be ESTABLISHED, have %+v", connection)
+			}
+			serverConnections++
+		}
+	}
+
+	clientConnections := 0
 	for _, connection := range c {
-		if connection.Status == "ESTABLISHED" && (connection.Laddr.IP == tcpServerAddrIP && connection.Laddr.Port == uint32(tcpServerAddrPort)) || (connection.Raddr.IP == tcpServerAddrIP && connection.Raddr.Port == uint32(tcpServerAddrPort)) {
+		if connection.Raddr.IP == tcpServerAddrIP && connection.Raddr.Port == uint32(tcpServerAddrPort) {
-			found++
+			if connection.Status != "ESTABLISHED" {
+				t.Fatalf("expected client connection to be ESTABLISHED, have %+v", connection)
+			}
+			clientConnections++
 		}
 	}
-	if found != 2 { // two established connections, one for the server, the other for the client
+
-		t.Errorf(fmt.Sprintf("wrong connections: %+v", c))
+	if serverConnections != 1 { // two established connections, one for the server, the other for the client
+		t.Fatalf("expected 1 server connection, have %d.\nDetails: %+v", serverConnections, c)
+	}
+
+	if clientConnections != 1 { // two established connections, one for the server, the other for the client
+		t.Fatalf("expected 1 server connection, have %d.\nDetails: %+v", clientConnections, c)
 	}
 }
 
@@ -749,34 +778,38 @@ func Test_Process_Environ(t *testing.T) {
 
 func Test_AllProcesses_cmdLine(t *testing.T) {
 	procs, err := Processes()
-	if err == nil {
+	skipIfNotImplementedErr(t, err)
-		for _, proc := range procs {
+	if err != nil {
-			var exeName string
+		t.Fatalf("getting processes error %v", err)
-			var cmdLine string
+	}
-
+	for _, proc := range procs {
-			exeName, _ = proc.Exe()
+		var exeName string
-			cmdLine, err = proc.Cmdline()
+		var cmdLine string
-			if err != nil {
-				cmdLine = "Error: " + err.Error()
-			}
 
-			t.Logf("Process #%v: Name: %v / CmdLine: %v\n", proc.Pid, exeName, cmdLine)
+		exeName, _ = proc.Exe()
+		cmdLine, err = proc.Cmdline()
+		if err != nil {
+			cmdLine = "Error: " + err.Error()
 		}
+
+		t.Logf("Process #%v: Name: %v / CmdLine: %v\n", proc.Pid, exeName, cmdLine)
 	}
 }
 
 func Test_AllProcesses_environ(t *testing.T) {
 	procs, err := Processes()
-	if err == nil {
+	skipIfNotImplementedErr(t, err)
-		for _, proc := range procs {
+	if err != nil {
-			exeName, _ := proc.Exe()
+		t.Fatalf("getting processes error %v", err)
-			environ, err := proc.Environ()
+	}
-			if err != nil {
+	for _, proc := range procs {
-				environ = []string{"Error: " + err.Error() }
+		exeName, _ := proc.Exe()
-			}
+		environ, err := proc.Environ()
-
+		if err != nil {
-			t.Logf("Process #%v: Name: %v / Environment Variables: %v\n", proc.Pid, exeName, environ)
+			environ = []string{"Error: " + err.Error()}
 		}
+
+		t.Logf("Process #%v: Name: %v / Environment Variables: %v\n", proc.Pid, exeName, environ)
 	}
 }