Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Party.Room constructor public #843

Open
kettanaito opened this issue Mar 14, 2024 · 2 comments
Open

Make Party.Room constructor public #843

kettanaito opened this issue Mar 14, 2024 · 2 comments

Comments

@kettanaito
Copy link

kettanaito commented Mar 14, 2024

Hi! Thank you so much for building PartyKit! I'm enjoying it a lot so far.

I'm building a simple app with PartyKit and Remix, and I'm testing it with the new ws API from MSW. Due to the way a PartyKit Server is structured, there's actually nothing that prevents me from using that server class in my tests to keep the server behavior consistent. I think that'd be amazing.

What I'm missing is a more developer-friendly way of creating a Party.Room instance. What I want to do is create a mock Room instance and implement its methods, like broadcast, with the ws API from MSW:

import { ws } from 'msw'
import GameServer from '~/party/game'

const game = ws.link('ws://localhost:3000/parties/game/index')

const gameServer = new GameServer({
  storage: new MockStorage(),
  // Whenever the actual server class broadcasts a message
  // to the room, forward that message to the "game" event handler
  // from MSW.
  broadcast: message => game.broadcast(message),
  ...
})

The problem is that there's no new Party.Room() runtime API. Constructing a matching object is rather tedious and leaks internal info like internalID. Ideally, the public interface is a constructor that accepts the minimal info needed to spawn a room. All the internals are pre-filled as a part of the Room class.

This way when I do requests in my tests, I intercept them with the game event handler and forward them to gameServer.onMessage():

game.on('connection', ({ client }) => {
  client.addEventListener('message', (event) => {
    // Whenever MSW intercepts the client sending an event,
    // forward it to the actual (test) PartyKit server instance.
    gameServer.onMessage(event.data)
  })
})

What I expect to happen here, is this:

  1. The WebSocket connection is intercepted and mocked by MSW (this part already works).
  2. MSW forward all outgoing client events to the gameServer, which is an instance of my actual PartyKit server just implemented with the mock APIs. This allows me to have the same state and execute the same logic as my production server would.
  3. I fire an event, the interceptor forwards it to gameServer.onMessage(). Since the broadcast method of the room is implemented using the game.broadcast() method, the connected client will receive an actual server response from the gameServer using the interceptor. No actual servers involved while keeping the actual server's behavior.

The expected usage API-wise would look like this:

import { Room, Storage } from 'partykit-somewhere'

const gameServer = new GameServer(new Room({
  storge: new Storage(),
  broadcast: message => interceptor.broadcast(message)
}))

Exposing the Storage API would also be nice since it's not a regular Map like I originally thought!

Would this be something you'd like to do? I think it can improve the testing strategy of PartyKit a lot.

@kettanaito
Copy link
Author

kettanaito commented Mar 14, 2024

I have a proof of concept functional at kettanaito/tug-o-war@00bdfa6! It reuses my GameServer PartyServer instance in a test to rely on the same behaviors I do in production. I believe this is the way to go regarding testing. Also touches on what we've discussed over our call, @threepointone, regarding how to emulate behaviors in tests.

If we can provide a better experience around the Room and Storage APIs that'd be incredible. Currently, I mock the Room object and it's not nice 😬:

https://github.com/kettanaito/tug-o-war/blob/00bdfa6a9ebb05ef8b2dc521d4d3adec734fb82b/app/components/tug-o-war.test.tsx#L11-L32

Now, I agree that a mock Room instance won't be a 1-1 replica of the production room. I think a certain degree of deviation is always acceptable in mocks. Providing the minimum, which is the storage, broadcast can cover most of the test cases (and the Room class can manage connections, getConnection(), getConnections(, etc. internally; I get a feeling that's what it does anyway now). The important bit is that an interceptor can forward client events (and server events from the actual PartyServer) to the tested code.

@kettanaito
Copy link
Author

Updates

Okay, after I've given this some more thought, I have concluded that reusing the actual PartyKit server for testing may not be the best idea. If you've constructed you server to support such a use case, I suppose you can do as you wish, but from the general testing practices perspective, I believe that's not a good way to set up a test.

My main motivation to rely on the actual WebSocket server was to reuse the existing behaviors of the server. Example: I send a pull event 10 times from the client and on the 10th time the server sends a finish event. This sounds like a logic crucial to have in a test. But in reality, it's not. It doesn't belong in the test.

That kind of logic doesn't belong in the test because it's not something the client knows or controls. The fact that the 10th pull must result in finish is server's logic, not the client's. Thus, to efficiently test the client in this scenario, what you should do is this:

  1. Test that doing an action results in the pull event being sent.
  2. Test that once the server sends a finish event, your client transitions into the correct state.

In the end, those are precisely the logic that the client defines and controls, and thus, also only logic that must be tested.

Now, you'd still test the actual 10 pulls -> complete in an end-to-end test with the actual/test WebSocket server running. My comments relate purely to integration-level testing.

With this conclusion, I don't think it's strictly necessary for PartyKit to expose primitives like Room for testing. I have achieved a decent integration test coverage without those primitives and without relying on PartyKit at all. Take a look at the example test suite here:

https://github.com/kettanaito/tug-o-war/blob/00872b6df7c1c20bf3676d579e209a13ada0210c/app/components/tug-o-war.test.tsx#L16-L18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant