gemmellr commented on issue #32: QPIDJMS-441: add proxy support URL: https://github.com/apache/qpid-jms/pull/32#issuecomment-544921618 The transport test is useful but more tests are definitely needed to ensure operation and that things dont get broken later. There arent any tests that actually exercise a connection being set up and configured the way an application would use it, so the connection process through the proxy isnt exercised, and the extension mechanism for passing through the handler from the factory is also untested. I believe currently the full-connection style tests are in the activem tests module for WS connections, everything else is in the client module (e.g ConnectionIntegrationTest, SslIntegrationTest). There also needs to be some SSL testing. The various create and clone etc tests in TransportOptionsTest need updated for the options change. A test doing failover would be needed to verify that works (I'd guess in that case it may additionally be necessary to require return of a new handler instance each time the handler Supplier get method is called, if so then that should perhaps be stated in the extension javadoc since Supplier itself notes it isnt a general requirement). If the extension mechanism is allowing for Socks proxy handlers to be passed then there should be tests for that too. The changes altered the opentracing-util dependency from provided to compile scope, that should be put back as it was.
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
