Okhttp: Make it easy to write WebSocket tests with MockWebServer

[gregschlom]

I noticed there are some very useful classes to allow testing web sockets code with MockWebServer in okhttp3.internal.ws.*, such as WebSocketRecorder.

It would be nice if these were public (along with anything else needed) so that OkHttp users can easily test their web sockets code with MockWebServer.

[iocanel]

For our own needs we have created a small wrapper around MockWebServer: https://github.com/fabric8io/mockwebserver. This wrapper actually provides a DSL and it does make it easy to work with websockets:

For example:

DefaultMockServer server = new DefaultMockServer();
server.expect().get().withPath("/api/v1/users/shell")
        .andUpgradeToWebSocket()
        .open()
             .waitFor(1000).andEmit("#")
            .expect("create root").andEmit("CREATED").once()
            .expect("delete root").andEmit("DELETED").once()
        .done()
        .once();

server.start();

[jhansche]

@swankjesse Do you have any plans for this? I don't mind making a PR, but that would probably be limited to essentially moving it into mockwebserver or okhttp-testing-support (which are released publicly).

Did you have something else in mind for this?

[swankjesse]

Designing some nicer APIs than what’s currently hacked in.

[PhilLab]

It also seems that WebSockets and Dispatchers don't work well together. Either I am making a mistake or they are not meant to be combined

class MyCustomDispatcher extends Dispatcher {
    @Override
    public MockResponse dispatch(RecordedRequest request) {
        if(request.getPath().startsWith("/My/Websocket/Path")) {
            return new MockResponse()
                .withWebSocketUpgrade(new WebSocketListener() {});
        } else {
            Log.v(TAG, "Not mocked request: " + request.getPath());
            return new MockResponse().setResponseCode(404);
        }
    }
}

After the above dispatcher, the MockWebServer calls MockWebServer.handleWebSocketUpgrade().
https://github.com/square/okhttp/blob/a912753b0979703c407ca6bb81c14f391930a1b4/mockwebserver/src/main/java/okhttp3/mockwebserver/MockWebServer.java#L581-L582

Inside of it a fancyRequest is built but without taking the path into account, so the dispatched "/My/Websocket/Path" is lost
https://github.com/square/okhttp/blob/a912753b0979703c407ca6bb81c14f391930a1b4/mockwebserver/src/main/java/okhttp3/mockwebserver/MockWebServer.java#L716-L719

The above dispatcher then reacts with a 404.

When registering a WebsocketListener which directly sends a message in onOpen (or if we have another thread sending something to the websocket, it does not matter):

class MyCustomDispatcher extends Dispatcher {
    @Override
    public MockResponse dispatch(RecordedRequest request) {
        if(request.getPath().startsWith("/My/Websocket/Path")) {
            WebsocketListener listener = new WebSocketListener() {
                @Override
                public void onOpen(WebSocket webSocket, Response response) {
                    Log.i(TAG, "onOpen: " + response);
                    webSocket.send("Test message");
                }
                @Override
                public void onFailure(WebSocket webSocket, Throwable t, @Nullable Response response) {
                    Log.i(TAG, "onFailure: " + t + ", " + response);
                }
            });
            return new MockResponse()
                .withWebSocketUpgrade(listener);
          } else {
            Log.v(TAG, "Not mocked request: " + request.getPath());
             return new MockResponse().setResponseCode(404);
          }
    }
}

this is the log output showing the path being stripped away

> onOpen: Response{protocol=http/1.1, code=101, message=Switching Protocols, url=http://localhost:41094/}
> Not mocked request: /
> MockWebServer[41094] received request: GET / HTTP/1.1 and responded: HTTP/1.1 404 Client Error
> onFailure: java.net.ProtocolException: Expected HTTP 101 response but was '404 Client Error', Response{protocol=http/1.1, code=404, message=Client Error, url=http://localhost:41094/}

[AOrobator]

Any progress on surfacing something like WebSocketRecorder? What are some of the issues with the current API design?

[yschimke]

MockWebServer should be it's own project, not a priority while part of OkHttp.