Tests added, critical Sonar complains resolved #59
Conversation
BaseWssFactoryBuilder and BaseWssSocketBUilder updated to be more flexible and testable Tests added for wss/* classes
Codecov Report
@@ Coverage Diff @@
## master #59 +/- ##
============================================
+ Coverage 51.04% 52.71% +1.66%
- Complexity 759 785 +26
============================================
Files 180 180
Lines 3048 3060 +12
Branches 218 221 +3
============================================
+ Hits 1556 1613 +57
+ Misses 1407 1360 -47
- Partials 85 87 +2 |
There was a problem hiding this comment.
The test names could use some work. I like to name them after the following format: unitUnderTest(method)_setup_expectedBehaviour.
This name convention is easier if you reduce the amount of asserts (preferably one), which in turn makes it easier to quickly identify what exactly is broke.
| @@ -154,7 +154,10 @@ protected Message parse(Object json) { | |||
| ((CallErrorMessage) message).setErrorCode(array.get(INDEX_CALLERROR_ERRORCODE).getAsString()); | |||
| ((CallErrorMessage) message).setErrorDescription(array.get(INDEX_CALLERROR_DESCRIPTION).getAsString()); | |||
| ((CallErrorMessage) message).setRawPayload(array.get(INDEX_CALLERROR_PAYLOAD).toString()); | |||
| } else { | |||
| throw new IllegalArgumentException("Unknown message type"); | |||
There was a problem hiding this comment.
I'd like to suggest you to add JSON body to the log.
| if(sslSocketFactory == null) { | ||
| throw new IllegalStateException("sslSocketFactory must be set"); | ||
| } | ||
|
|
||
| if(complete) { |
There was a problem hiding this comment.
Put space before '(', please. There and in other places.
There was a problem hiding this comment.
I would propose to define reference for the project for a particular codestyle so it could be formatted automatically in accrodance (e.g. https://google.github.io/styleguide/javaguide.html)
| Socket expectedSocket = mock(Socket.class); | ||
| Socket socket = mock(Socket.class); | ||
| when(socket.isBound()).thenReturn(false); | ||
| when(sslSocketFactory.createSocket(socket,"fake", 101, false)).thenReturn(expectedSocket); |
There was a problem hiding this comment.
Please, put space after comma. There and in other places.
There was a problem hiding this comment.
Done for the particular case. In general I would propose to define reference for the project for a particular codestyle so it could be formatted automatically in accrodance (e.g. https://google.github.io/styleguide/javaguide.html)
|
What do you think @eupakhomov ? |
Logging of message body added to JSON communicator in case of unknown message type Cosmetics
|
Sorry for the delay. Tests' names are fixed in wss package (each test uses exactly one assertion sometimes with a few verifications as well). For code style I would propose to define a common approach. |
BaseWssFactoryBuilder and BaseWssSocketBUilder updated to be more flexible and testable
Tests added for wss/* classes