[GitHub] [tinkerpop] divijvaidya commented on pull request #1594: Gremlin-Go GA - Preview Ready

[GitBox <gi...@apache.org>]

divijvaidya commented on pull request #1594:
URL: https://github.com/apache/tinkerpop/pull/1594#issuecomment-1079042384


   > > We are missing integration tests against the server (unless I missed something), which can validate the client behaviour in scenario such as:
   > 
   > I'm not sure that we can do this right now. The only driver that has any tests like this is Java and that is because the test setup is such that we can easily start/stop/reconfigure Gremlin Server. There is no such mechanism to do so for non-JVM language variants. I do think this sort of testing is important but it will imply a whole range of test framework development to allow this to happen and I'm not sure what that would look like as it would require per-test mechanisms for that start/stop/reconfigure functionality.
   > 
   > Unless there is an easy way to do this that I'm not aware of I'd say that such testing would have to be part of a different/larger body of work which builds a Gherkin-like integration test framework for driver behaviors that would cover connection management, sessions, etc.
   
   My main concern is that GA status for a component here in this project would mean that users can start using this in production scenarios. However, we can't confidently say that the client is production ready unless we have tested it on various scenarios involving client-server interaction.
   
   > The only driver that has any tests like this is Java and that is because the test setup is such that we can easily start/stop/reconfigure Gremlin Server. There is no such mechanism to do so for non-JVM language variants. 
   
   I understand what we did not have integration tests for other non-java drivers in the past. But we don't have to keep following the same practices from the past if they were incorrect. Given the scope of usage TinkerPop clients has across the industry, I would argue that we need to release well tested and stable software if we are making something GA.
   
   We can still merge this PR into mainline without integration tests and call it Beta (not recommended for production usage) until we have sufficient testing in place. But I don't agree with calling this ready for production use (GA / preview-GA).
   
   Now, as far as how to do the technical implementation goes, we can either do it against the TinkerPop server or a mock WebSocket server. The ultimate aim is to assert the client-server contract, e.g. does the client respect a close WebSocket frame from the server correctly.
   
   **Option A: With live server**
   
   1. Write an API in the server available in debug mode which will trigger the specific test behaviour we want, e.g. close the WS connection with a WS frame.
   
   2. Use a shell script to start the server and then run a go file containing the integration test. The integration test assumes that a server is running on port 8182
   
   3. Invoke the "test" API from point 1 to make the server do the desired behaviour. 
   
   **Option B: With mock WS server** (All the scenarios that I wrote in the PR comment can be handled by a mock WS server)
   
   Gorilla WebSocket library (the same one that client uses in this PR) already has example tests on how to mock a server. See: https://github.com/gorilla/websocket/blob/master/client_server_test.go#L62 
   
   we can mock a server with WebSocket connection similarly and make it send a close websocket to see how the client reacts and assert the reaction. Similarly for other test scenarios I mentioned, we can mock the connection "disconnect" and can mock the "WS handshake timeout" using the mocks. We don't really need a full TinkerPop server to do this.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org