533: Create explicitly a mock socket #629
Conversation
|
@essobedo Thanks, I will find someone to review this PR soon |
|
@krzyk please review, thanks |
| final Socket socket = Mockito.mock(Socket.class); | ||
| Mockito.when(socket.getInputStream()).thenReturn( | ||
| new ByteArrayInputStream( | ||
| private static BkBasicTest.MkSocket createMockSocket() throws IOException { |
There was a problem hiding this comment.
@essobedo maybe it would be better to inline this method
There was a problem hiding this comment.
@krzyk I'm not sure I understand your remark here. Could please clarify?
There was a problem hiding this comment.
@essobedo remove createMockSocket method and use the class directly (using new MkSocket())
|
@essobedo please see my comments |
|
@krzyk thx for the review and the remarks. Here is a new version of my PR, please note that I finally kept the method createMockSocket because it used in several places in the test class and the input stream of the socket is specific to those tests and as the goal is to make MkSocket reusable, I had to add a new parameter to the constructor to specify the expected input stream. |
| */ | ||
| public MkSocket(final InputStream input) throws IOException { | ||
| super(); | ||
| this.address = InetAddress.getLocalHost(); |
There was a problem hiding this comment.
@essobedo use InetAddress.getLoopbackAddress() and remove throws IOException
|
@essobedo one additional comment |
|
@krzyk good one, here is the new version of the PR |
|
@rultor merge pls |
|
@rultor try to merge |
|
@elenavolokhova please, review this issue for QA compliance, as per par.24 |
|
@rultor please deploy |
|
@davvd Looks good! |
@elenavolokhova thank you :) |
| @@ -64,8 +62,6 @@ | |||
| * persistent connections. Would be great to implement | |||
| * this feature. BkBasic.accept should handle more | |||
| * than one HTTP request in one connection. | |||
| * @todo #516:30min It will be nice to refactor tests with Socket usage and | |||
| * replace them to real statements. See usage of BkBasicTest.createMockSocket. | |||
| * @todo #516:15min Move header names from BkBasic to public constants. | |||
There was a problem hiding this comment.
@essobedo ignore my comment
my bad you didn't add the puzzle
|
@krzyk added 10 mins to @elenavolokhova, for QA review, payment code is done, I added 23 mins in payment review comments (c=8) added as a bonus +23 added to your rating, at the moment it is: +5953 |
Fix for #533:
Followed the principale described here by replacing a mock created with Mockito with an explicit mock Socket called MkSocket.