add the mock transport and remove asClient* helpers #268
Conversation
jackyzha0
requested review from
bradymadden97
and removed request for
a team
| @@ -255,177 +239,6 @@ function dummyCtx<State>( | |||
| }; | |||
| } | |||
|
|
|||
| export function asClientRpc< | |||
__tests__/fixtures/mockTransport.ts
Outdated
|
|
||
| const transports: Array<MockClientTransport | MockServerTransport> = []; | ||
| class MockClientTransport extends ClientTransport<InMemoryConnection> { | ||
| async createNewOutgoingConnection(): Promise<InMemoryConnection> { |
There was a problem hiding this comment.
Feels like to should be accepted here and then the "MockTransport" should only care about connections targeted at its clientId
__tests__/fixtures/mockTransport.ts
Outdated
| getServerTransport: (handshakeOptions) => { | ||
| const serverTransport = new MockServerTransport('SERVER', opts?.server); |
There was a problem hiding this comment.
| getServerTransport: (handshakeOptions) => { | |
| const serverTransport = new MockServerTransport('SERVER', opts?.server); | |
| getServerTransport: (id = `SERVER${nanoid()}`, handshakeOptions) => { | |
| const serverTransport = new MockServerTransport(id, opts?.server); |
__tests__/fixtures/mockTransport.ts
Outdated
| for (const conn of Object.values(conns)) { | ||
| if (conn.handled) { | ||
| continue; | ||
| } | ||
|
|
||
| // if we find one, handle it | ||
| conn.handled = true; | ||
| const connection = new InMemoryConnection(conn.serverToClient); | ||
|
|
||
| this.handleConnection(connection); |
There was a problem hiding this comment.
seems strange that you can create many server transports but not have control over which one clients are connecting to.
There was a problem hiding this comment.
right now, our transport fixture just assumes theres only one server (our ws mock
river/__tests__/fixtures/transports.ts
Line 52 in 1bc1ce9
i think its ok as none of our tests assume multiple server transports? i can also refactor both fixtures to allow for multiple transports but seems a bit YAGNI
__tests__/fixtures/mockTransport.ts
Outdated
| clientTransport.extendHandshake(handshakeOptions); | ||
| } | ||
|
|
||
| transports.push(clientTransport); |
There was a problem hiding this comment.
why are we storing these here? seems like the only place we're using transports we're filtering out clients.
__tests__/fixtures/mockTransport.ts
Outdated
| async restartServer() { | ||
| for (const transport of transports) { | ||
| if (transport.clientId !== 'SERVER') continue; | ||
| transport.close(); |
There was a problem hiding this comment.
is this a restart? isn't close destructive?
There was a problem hiding this comment.
yeah we destroy the old one and make an entirely new one, transports dont have the ability to 'restart' so we emulate it losing state by doing this
__tests__/fixtures/mockTransport.ts
Outdated
| simulatePhantomDisconnect() { | ||
| for (const conn of Object.values(connections.get())) { | ||
| conn.serverToClient.pause(); | ||
| } | ||
| }, |
There was a problem hiding this comment.
you dont, this is just following the interface in
river/__tests__/fixtures/transports.ts
Line 32 in 1bc1ce9
we can add the ability to do that if we need it?
Why
asClient*style test helpers relied on emulating the logic in router precisely and this can go out of date/drift as changes are madeWhat changed
Versioning