vert-x3 / vertx-tcp-eventbus-bridge

Apache License 2.0
49 stars 44 forks source link

Fix tests for experimental JSON-RPC bridge #68

Closed lucifer4j closed 2 years ago

lucifer4j commented 2 years ago

56 describes the rationale for adding a JSON-RPC bridge. The experimental/jsonrpc branch has an initial implementation for the bridge but some of the tests are failing. This PR fixes those tests.

Many of the failing tests were because the tests were not expecting some messages but the bridge was sending them. For instance, when using the Vert.x protocol bridge the client does not receive a message confirming registration but the JSON-RPC spec mandates a response for all requests (except notifications) so the JSON-RPC protocol bridge responds with a ack for register message. The test being modelled on lines of the Vertx protocol tests does not expect this message and hence fails.

This PR updates the tests to handle such messages. I have tried to add comments to explain the logic behind the changes and fixed each test in a separate commit with a somewhat detailed commit message. I hope that helps in reviewing the changes and understanding the rationale behind the fixes.

Apart from this issue, there is one implementation issue in the JSON-RPC bridge - namely, handling response from client to bridge marking an error. The JSON-RPC bridge expect the message to be similar to Vertx one and hence does not handle it well. I have updated the implementation there to expect a JSON-RPC looking message. However, that format is not set in stone since I don't think the spec covers the case of a client sending a error message to the bridge.

pmlopes commented 2 years ago

@lucifer4j great work :+1: ! I'll review this soon!