From 79896a1d265c7188f3fb58fb4b23df809fd38164 Mon Sep 17 00:00:00 2001 From: Tom Barker Date: Tue, 17 Aug 2021 11:41:57 -0400 Subject: [PATCH] Fix for Test_Connections currently fails intermittently on Linux (and maybe other OSs). On inspection, Go closes TCP connections when they go out of scope and are garbage collected. I've re-written Test_Connections() to avoid closing until the test has finished. Other changes: - Close connections properly in TestConnections so running go test -times=N works. - t.Skip() can not be called inside a test go-routine. - Make sure that Test_AllProcesses_cmdLine doesn't ignore failures. - Some minor style changes motivated by staticcheck warnings. --- process/process_linux.go | 7 +-- process/process_test.go | 108 +++++++++++++++++++----------------- v3/process/process_linux.go | 7 +-- v3/process/process_test.go | 108 +++++++++++++++++++----------------- 4 files changed, 120 insertions(+), 110 deletions(-) diff --git a/process/process_linux.go b/process/process_linux.go index ead99adaa..de742fbde 100644 --- a/process/process_linux.go +++ b/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 true - } - return false + return r == '\u0000' }) return strings.Join(ret, " "), nil diff --git a/process/process_test.go b/process/process_test.go index 5eaa03928..d0c5dc7d3 100644 --- a/process/process_test.go +++ b/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,46 +498,50 @@ 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) + } + 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() + + _, 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 - tcpServerAddrIP := strings.Split(tcpServerAddr, ":")[0] - tcpServerAddrPort, err := strconv.ParseUint(strings.Split(tcpServerAddr, ":")[1], 10, 32) + conn, err := net.Dial("tcp", tcpServerAddr) if err != nil { - t.Errorf("unable to parse tcpServerAddr port: %v", err) + t.Fatalf("unable to dial %v: %v", tcpServerAddr, err) } + defer conn.Close() + 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 for _, connection := range c { @@ -546,7 +550,7 @@ func Test_Connections(t *testing.T) { } } if found != 2 { // two established connections, one for the server, the other for the client - t.Errorf(fmt.Sprintf("wrong connections: %+v", c)) + t.Fatalf("wrong connections: %+v", c) } } @@ -747,34 +751,38 @@ func Test_Process_Environ(t *testing.T) { func Test_AllProcesses_cmdLine(t *testing.T) { procs, err := Processes() - if err == nil { - for _, proc := range procs { - var exeName string - var cmdLine string - - 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) + skipIfNotImplementedErr(t, err) + if err != nil { + t.Fatalf("getting processes error %v", err) + } + for _, proc := range procs { + var exeName string + var cmdLine string + + 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 { - for _, proc := range procs { - exeName, _ := proc.Exe() - environ, err := proc.Environ() - if err != nil { - environ = []string{"Error: " + err.Error() } - } - - t.Logf("Process #%v: Name: %v / Environment Variables: %v\n", proc.Pid, exeName, environ) + skipIfNotImplementedErr(t, err) + if err != nil { + t.Fatalf("getting processes error %v", err) + } + for _, proc := range procs { + exeName, _ := proc.Exe() + environ, err := proc.Environ() + if err != nil { + environ = []string{"Error: " + err.Error()} } + + t.Logf("Process #%v: Name: %v / Environment Variables: %v\n", proc.Pid, exeName, environ) } } diff --git a/v3/process/process_linux.go b/v3/process/process_linux.go index 90f61e9c6..a424e0033 100644 --- a/v3/process/process_linux.go +++ b/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 true - } - return false + return r == '\u0000' }) return strings.Join(ret, " "), nil diff --git a/v3/process/process_test.go b/v3/process/process_test.go index 3d5051a2d..df31b028f 100644 --- a/v3/process/process_test.go +++ b/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,46 +500,50 @@ 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) + } + 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() + + _, 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 - tcpServerAddrIP := strings.Split(tcpServerAddr, ":")[0] - tcpServerAddrPort, err := strconv.ParseUint(strings.Split(tcpServerAddr, ":")[1], 10, 32) + conn, err := net.Dial("tcp", tcpServerAddr) if err != nil { - t.Errorf("unable to parse tcpServerAddr port: %v", err) + t.Fatalf("unable to dial %v: %v", tcpServerAddr, err) } + defer conn.Close() + 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 for _, connection := range c { @@ -548,7 +552,7 @@ func Test_Connections(t *testing.T) { } } if found != 2 { // two established connections, one for the server, the other for the client - t.Errorf(fmt.Sprintf("wrong connections: %+v", c)) + t.Fatalf("wrong connections: %+v", c) } } @@ -749,34 +753,38 @@ func Test_Process_Environ(t *testing.T) { func Test_AllProcesses_cmdLine(t *testing.T) { procs, err := Processes() - if err == nil { - for _, proc := range procs { - var exeName string - var cmdLine string - - 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) + skipIfNotImplementedErr(t, err) + if err != nil { + t.Fatalf("getting processes error %v", err) + } + for _, proc := range procs { + var exeName string + var cmdLine string + + 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 { - for _, proc := range procs { - exeName, _ := proc.Exe() - environ, err := proc.Environ() - if err != nil { - environ = []string{"Error: " + err.Error() } - } - - t.Logf("Process #%v: Name: %v / Environment Variables: %v\n", proc.Pid, exeName, environ) + skipIfNotImplementedErr(t, err) + if err != nil { + t.Fatalf("getting processes error %v", err) + } + for _, proc := range procs { + exeName, _ := proc.Exe() + environ, err := proc.Environ() + if err != nil { + environ = []string{"Error: " + err.Error()} } + + t.Logf("Process #%v: Name: %v / Environment Variables: %v\n", proc.Pid, exeName, environ) } }