Skip to content

Commit

Permalink
Fix TestGettingTrackingData flakiness (open-telemetry#27927)
Browse files Browse the repository at this point in the history
**Description:**
Fix flakiness in the
`TestGettingTrackingData/Timeout_waiting_for_response` test. The root
cause of the issue is that the `client` and `server` are connected to
same pipe and if the server attempts to read after the client is closed
it reports error EOF and the test code expects no error from the read
operation. The issue is not specific to Windows. The fix was to give
control to each test case of the read and write functions so this type
of synchronization issue can be properly handled while keeping the test
code coverage.

**Link to tracking Issue:**
Fix open-telemetry#25188

**Testing:**
Multiple local runs with various cpu settings.

**Documentation:** N/A

Co-authored-by: Alex Boten <[email protected]>
  • Loading branch information
2 people authored and jmsnll committed Nov 12, 2023
1 parent ef9c45b commit 95774e0
Showing 1 changed file with 28 additions and 16 deletions.
44 changes: 28 additions & 16 deletions receiver/chronyreceiver/internal/chrony/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
"github.com/stretchr/testify/require"
)

func newMockConn(tb testing.TB, handler func(net.Conn) error) net.Conn {
func newMockConn(tb testing.TB, serverReaderFn, serverWriterFn func(net.Conn) error) net.Conn {
client, server := net.Pipe()

var wg sync.WaitGroup
Expand All @@ -32,8 +32,19 @@ func newMockConn(tb testing.TB, handler func(net.Conn) error) net.Conn {
go func() {
defer wg.Done()

assert.NoError(tb, binary.Read(server, binary.BigEndian, &requestTrackingContent{}), "Must not error when reading binary data")
assert.NoError(tb, handler(server), "Must not error when processing request")
if serverReaderFn == nil {
serverReaderFn = func(conn net.Conn) error {
return binary.Read(conn, binary.BigEndian, &requestTrackingContent{})
}
}
assert.NoError(tb, serverReaderFn(server), "Must not error when reading binary data")

if serverWriterFn == nil {
serverWriterFn = func(conn net.Conn) error {
return nil
}
}
assert.NoError(tb, serverWriterFn(server), "Must not error when processing request")
}()
return client
}
Expand Down Expand Up @@ -74,17 +85,18 @@ func TestGettingTrackingData(t *testing.T) {
t.Parallel()

tests := []struct {
scenario string
handler func(conn net.Conn) error
dialTime time.Duration
timeout time.Duration
data *Tracking
err error
scenario string
serverReaderFn func(conn net.Conn) error
serverWriterFn func(conn net.Conn) error
dialTime time.Duration
timeout time.Duration
data *Tracking
err error
}{
{
scenario: "Successful read binary from socket",
timeout: 10 * time.Second,
handler: func(conn net.Conn) error {
serverWriterFn: func(conn net.Conn) error {
type response struct {
ReplyHead
replyTrackingContent
Expand Down Expand Up @@ -142,15 +154,15 @@ func TestGettingTrackingData(t *testing.T) {
scenario: "Timeout waiting for dial",
timeout: 10 * time.Millisecond,
dialTime: 100 * time.Millisecond,
handler: func(conn net.Conn) error {
serverReaderFn: func(conn net.Conn) error {
return nil
},
err: os.ErrDeadlineExceeded,
},
{
scenario: "Timeout waiting for response",
timeout: 10 * time.Millisecond,
handler: func(conn net.Conn) error {
serverReaderFn: func(conn net.Conn) error {
time.Sleep(100 * time.Millisecond)
return nil
},
Expand All @@ -160,7 +172,7 @@ func TestGettingTrackingData(t *testing.T) {
scenario: "Timeout waiting for response because of slow dial",
timeout: 100 * time.Millisecond,
dialTime: 90 * time.Millisecond,
handler: func(conn net.Conn) error {
serverWriterFn: func(conn net.Conn) error {
time.Sleep(20 * time.Millisecond)
return nil
},
Expand All @@ -169,7 +181,7 @@ func TestGettingTrackingData(t *testing.T) {
{
scenario: "invalid status returned",
timeout: 5 * time.Second,
handler: func(conn net.Conn) error {
serverWriterFn: func(conn net.Conn) error {
resp := &ReplyHead{
Version: 6,
Status: 1,
Expand All @@ -182,7 +194,7 @@ func TestGettingTrackingData(t *testing.T) {
{
scenario: "invalid status command",
timeout: 5 * time.Second,
handler: func(conn net.Conn) error {
serverWriterFn: func(conn net.Conn) error {
resp := &ReplyHead{
Version: 6,
Status: successfulRequest,
Expand All @@ -205,7 +217,7 @@ func TestGettingTrackingData(t *testing.T) {
return nil, os.ErrDeadlineExceeded
}

return newMockConn(t, tc.handler), nil
return newMockConn(t, tc.serverReaderFn, tc.serverWriterFn), nil
}
})
require.NoError(t, err, "Must not error when creating client")
Expand Down

0 comments on commit 95774e0

Please sign in to comment.