feat(dismock): Gateway Mocking

[superloach]

Is your feature request related to a problem? Please describe. I want to be able to mock UpdateStatus calls.

Describe the solution you'd like I'd like a mock Gateway to be implemented, similar to the mock Session, which allows receiving events from the client. Sending isn't really necessary, since you can just call handlers with event structures.

Describe alternatives you've considered I've considered passing an interface to my functions which make UpdateStatus calls, but that just feels weird.

[superloach]

It looks like it may be possible to plug a dummy wsutil.Connection into the Gateway and handle events from Listen. Not sure though.

Edit: Got that backwards, we need to handle events from Send.

[superloach]

I got a basic idea working with a wsutil.Connection: https://gist.github.com/superloach/a04c8b1e8678ed61be62f519bbb3d679

This doesn't wrap anything in a "mocker" yet, but it does prove that this is fairly easy to do just by mocking the WS behavior.

[mavolin]

This looks good so far!

Below are a few thoughts, before you get into implementing this. These are just initial ideas, so feel free to comment on any of those. Quick note beforehand: Because v2 isn't released, you don't need to worry about breaking changes.

Package Layout

Seeing as dismock is getting more and more complicated, splitting up packages might be a good idea. I'd propose to do the following:

pkg/dismock
pkg/dismock/api
pkg/dismock/gateway

Using this package layout, there would be a dismock.Mocker that basically just embeds an api.Mocker and a gateway.Mocker. Only the constructor functions remain in package dismock, everything else that's in there right now gets moved to package api.

Edit: dismock.Mocker.Eval() should probably be kept, and delegate to api.Mocker.Eval() and gateway.Mocker.Eval().

Order of Requests

Similar to API mocks, on first thought I think we should enforce order per command type, e.g. if the UpdateStatus command a gets added first, the arikawa gateway must request it before UpdateStatus b to not cause a test fail. However, any other command may be requested in between the two (or before), even if it was added before or after the UpdateStatus commands. Effectively, this means something like type Mocker struct { commands map[gateway.OPCode]interface{} }, where interface is the event's data.

Inferring Opcode And Event Name

For convenience, both opcode and the event name should automatically be inferred when awaiting or requesting something. Opcodes can be manually inferred, e.g. through a map[reflect.Type]gateway.OPCode. Event names are a bit simpler: Arikawa provides gateway.EventCreator from which we can build a map[reflect.Type]string during init(). Seeing as the EventCreator returns pointers, we should probably use the .Elem() type. We can then do the same for the type of the event struct passed to the mocker, when looking up the event name.

Still, I think we should probably have a low-level method which takes in a custom opcode and event name. See below for that.

Methods provided by gateway.Mocker

The gateway mocker should have the following exposed functions. The names are not great, so feel free to change them if you think of something better, but this is more about the functionality anyway.

// asserts that a command with the passed opcode and data is received
func (*Mocker) MockCommand(opCode gateway.OPCode, data interface{})

// sends an event with the passed opcode, name, and data through the gateway.
func (*Mocker) MockEvent(opCode gateway.OPCode, name string, data interface{})

// asserts that a command with the passed data is received; opcode gets inferred, panic if unknown
func (*Mocker) ExpectCommand(command interface{})

// sends an event with the passed data through the gateway; opcode and name gets inferred, panic if unknown
func (*Mocker) SendEvent(event interface{})

[mavolin]

Also, this is probably because of the gist, but you can use testify for testing.

[superloach]

I've started poking around at rearranging the packages, and it seems it may be a better idea to make a proper mock WS server, since that's how api does it.

Edit: this would also allow other libs to use Dismock for gateway testing.

[superloach]

Also, something to note - we can't embed an api.Mocker and a gateway.Mocker due to field conflicts. I feel like gateway mocking will be a lot less frequently used, and shouldn't be embedded in the main Mocker type. Instead, it can just be added on, something like this:

m, s := dismock.NewSession(t)
m.Gateway, s.Gateway = dismock.NewGateway(t)

The Gateway field will be nil by default, and Eval will check this to see if it was manually added.

[superloach]

If we're enabling users to mock events like READY, it might be more correct to actually enable the Cabinet in a State, since that's normal expected behavior when gateway events are coming in. But I suppose that can be up to the user, if gateway mocking won't be enabled by default.

[superloach]

Regarding this snippet:

type Mocker struct {
    commands map[gateway.OPCode]interface{}
}

I think we would want something more like map[gateway.OPCode][]gateway.Event, if we want to mock events in sequence. Or maybe I'm thinking about this the wrong way.

[mavolin]

I've started poking around at rearranging the packages, and it seems it may be a better idea to make a proper mock WS server, since that's how api does it.

Edit: this would also allow other libs to use Dismock for gateway testing.

Yes, I would prefer this over just implementing the interface, but if the WS turns out to be too complicated, you can fall back to just implementing the interface.

[mavolin]

Also, something to note - we can't embed an api.Mocker and a gateway.Mocker due to field conflicts. I feel like gateway mocking will be a lot less frequently used, and shouldn't be embedded in the main Mocker type. Instead, it can just be added on, something like this:

m, s := dismock.NewSession(t)
m.Gateway, s.Gateway = dismock.NewGateway(t)

The Gateway field will be nil by default, and Eval will check this to see if it was manually added.

Forgot about the namespace collision, which indeed makes that impossible. However, I don't quite like setting Gateway to nil by default. What do you think of filling it, and just adding an OpenGateway() method to the mocker? Sessions and States wouldn't even have to call that, as we can just replace the wsutil.Websocket of the gateway, with one that calls OpenGateway() on Websocket.Dial(). That way only non-arikawa libs would even have to take this extra step.

[mavolin]

Adding to that, we could also still split up into api and gateway, and then embed api.Mocker and add delegate methods for gateway.Mocker, as it shouldn't have that many methods anyway. This is not super elegant, but I think better than having a separate Gateway field, although I'm not entirely sure, so feel free to comment on this.

[mavolin]

If we're enabling users to mock events like READY, it might be more correct to actually enable the Cabinet in a State, since that's normal expected behavior when gateway events are coming in. But I suppose that can be up to the user, if gateway mocking won't be enabled by default.

We should probably leave this to the user, as this would remove the testability of API calls (dismock isn't hit, because the elements are cached), and I would imagine this only being needed in a handful of scenarios, whereas the main use case of gateway mocks will probably be mocking commands.

[mavolin]

Regarding this snippet:

type Mocker struct {
    commands map[gateway.OPCode]interface{}
}

I think we would want something more like map[gateway.OPCode][]gateway.Event, if we want to mock events in sequence. Or maybe I'm thinking about this the wrong way.

Oh yeah, that was a typo. I'm honestly fine with either []interface{} or []gateway.Event. When I wrote that, I intended the interface{} to be the event's data, so the gateway.Event could be constructed from the map's key (opcode) and the serialized event data since all other fields are null.