Azure / go-amqp

AMQP 1.0 client library for Go.
https://github.com/Azure/go-amqp
MIT License
104 stars 56 forks source link

Test: fake.NetConn.Write() will block until a response has been sent #290

Closed jhendrixMSFT closed 1 year ago

jhendrixMSFT commented 1 year ago

This can cause spurious test failures like this one.

2023-05-05T00:18:56.3473981Z === RUN   TestNewSessionTimedOut
2023-05-05T00:18:56.3474395Z     conn_test.go:934: 
2023-05-05T00:18:56.3474766Z            Error Trace:    conn_test.go:934
2023-05-05T00:18:56.3475192Z            Error:          Target error should be in err chain:
2023-05-05T00:18:56.3475597Z                            expected: "context deadline exceeded"
2023-05-05T00:18:56.3475950Z                            in chain: 
2023-05-05T00:18:56.3476314Z            Test:           TestNewSessionTimedOut

Internally, Write() calls the registered response handler, and if the handler sleeps e.g. to simulate a slow response, that delay is reflected in the call to c.writeFrame() from Conn.connWriter instead of asynchronously.

What needs to happen is, in fake.NetConn.Write(), the incoming bytes should be queued up for processing (i.e. dispatching to n.resp()) on a separate goroutine so that any delays aren't reflected in the call to c.writeFrame().

Will also need to consider the case where a test simulates a write error. In that case, don't dispatch to the processing goroutine.

jhendrixMSFT commented 1 year ago

See also

--- FAIL: TestNewSenderTimedOut (0.12s)
    sender_test.go:1108:
                Error Trace:    sender_test.go:1108
                Error:          Target error should be in err chain:
                                expected: "context deadline exceeded"
                                in chain:
                Test:           TestNewSenderTimedOut