Skip to content

Commit

Permalink
Better isolate the tests under TestHandleReturn_regression
Browse files Browse the repository at this point in the history
Previously, these tests used the same connection for both tests.
This fixes intermittent test failures on the second test, as discussed
in capnproto#318.

I think, in principle, the tests *should* pass in that case anyway,
but:

1. Keeping the cases isolated makes this easier to understand
2. This way we can (and now do) run the tests in parallel
3. While we should better test the scenarios where:
   - a connection is used after a bootstrap fails
   - bootstrap is called more than once
   - etc.
   Doing so without tests dedicated to those things will be very
   difficult to maintain; we could add some but that should be a
   separate (low priority) task.
  • Loading branch information
zenhack committed Oct 13, 2022
1 parent 066ffec commit 19876b9
Showing 1 changed file with 48 additions and 38 deletions.
86 changes: 48 additions & 38 deletions rpc/level0_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1772,52 +1772,62 @@ func TestSendCancel(t *testing.T) {
func TestHandleReturn_regression(t *testing.T) {
t.Parallel()

p1, p2 := transport.NewPipe(1)

srv := testcp.PingPong_ServerToClient(pingPongServer{})
conn1 := rpc.NewConn(rpc.NewTransport(p2), &rpc.Options{
BootstrapClient: capnp.Client(srv),
})
defer conn1.Close()
// Common setup for the tests below. Creates a connection with
// a suitable bootstrap interface on one end, then passes the
// other end of the connection to the callback.
withConn := func(f func(*rpc.Conn)) {
p1, p2 := transport.NewPipe(1)

srv := testcp.PingPong_ServerToClient(pingPongServer{})
conn1 := rpc.NewConn(rpc.NewTransport(p2), &rpc.Options{
BootstrapClient: capnp.Client(srv),
})
defer conn1.Close()

conn2 := rpc.NewConn(rpc.NewTransport(p1), nil)
defer conn2.Close()
conn2 := rpc.NewConn(rpc.NewTransport(p1), nil)
defer conn2.Close()
f(conn2)
}

t.Run("MethodCallWithExpiredContext", func(t *testing.T) {
pp := testcp.PingPong(conn2.Bootstrap(context.Background()))
defer pp.Release()

// create an EXPIRED context
ctx, cancel := context.WithCancel(context.Background())
cancel()

f, release := pp.EchoNum(ctx, func(ps testcp.PingPong_echoNum_Params) error {
ps.SetN(42)
return nil
withConn(func(conn *rpc.Conn) {
pp := testcp.PingPong(conn.Bootstrap(context.Background()))
defer pp.Release()

// create an EXPIRED context
ctx, cancel := context.WithCancel(context.Background())
cancel()

f, release := pp.EchoNum(ctx, func(ps testcp.PingPong_echoNum_Params) error {
ps.SetN(42)
return nil
})
defer release()

_, err := f.Struct()
assert.ErrorIs(t, err, ctx.Err())
})
defer release()

_, err := f.Struct()
assert.ErrorIs(t, err, ctx.Err())
})

t.Run("BootstrapWithExpiredContext", func(t *testing.T) {
// create an EXPIRED context
ctx, cancel := context.WithCancel(context.Background())
cancel()

// NOTE: bootstrap with expired context
pp := testcp.PingPong(conn2.Bootstrap(ctx))
defer pp.Release()

f, release := pp.EchoNum(ctx, func(ps testcp.PingPong_echoNum_Params) error {
ps.SetN(42)
return nil
withConn(func(conn *rpc.Conn) {
// create an EXPIRED context
ctx, cancel := context.WithCancel(context.Background())
cancel()

// NOTE: bootstrap with expired context
pp := testcp.PingPong(conn.Bootstrap(ctx))
defer pp.Release()

f, release := pp.EchoNum(ctx, func(ps testcp.PingPong_echoNum_Params) error {
ps.SetN(42)
return nil
})
defer release()

_, err := f.Struct()
assert.ErrorIs(t, err, ctx.Err())
})
defer release()

_, err := f.Struct()
assert.ErrorIs(t, err, ctx.Err())
})
}

Expand Down

0 comments on commit 19876b9

Please sign in to comment.