suneet-s commented on a change in pull request #11529:
URL: https://github.com/apache/druid/pull/11529#discussion_r682211626
##########
File path:
core/src/test/java/org/apache/druid/data/input/HandlingInputRowIteratorTest.java
##########
@@ -81,58 +87,81 @@ public void throwsExceptionWhenYieldingNext()
public static class PresentRowTest
{
+ // Create variables for tracking behaviors of mock object
+
+ boolean unsuccessfulHandlerSuccessful;
+
+ // Create variables for tracking behaviors of mock object
+
+ boolean successfulHandlerSuccessful;
+
private static final InputRow INPUT_ROW1 = EasyMock.mock(InputRow.class);
private static final InputRow INPUT_ROW2 = EasyMock.mock(InputRow.class);
private static final List<InputRow> INPUT_ROWS = Arrays.asList(INPUT_ROW1,
INPUT_ROW2);
- private TestInputRowHandler successfulHandler;
- private TestInputRowHandler unsuccessfulHandler;
+ private InputRowHandler successfulHandler;
+ private InputRowHandler unsuccessfulHandler;
@Before
public void setup()
{
- successfulHandler = new TestInputRowHandler(true);
- unsuccessfulHandler = new TestInputRowHandler(false);
+ // Construct mock object
+ successfulHandler = mock(HandlingInputRowIterator.InputRowHandler.class);
+ // Constructing variables that used for tracking behavior of the mocking
+ // object.
+ successfulHandlerSuccessful = true;
+ // Method Stubs
+
when(successfulHandler.handle(any(InputRow.class))).thenAnswer((stubInvo) -> {
+ return successfulHandlerSuccessful;
+ });
+ // Construct mock object
+ unsuccessfulHandler =
mock(HandlingInputRowIterator.InputRowHandler.class);
+ // Constructing variables that used for tracking behavior of the mocking
+ // object.
+ unsuccessfulHandlerSuccessful = false;
+ // Method Stubs
+
when(unsuccessfulHandler.handle(any(InputRow.class))).thenAnswer((stubInvo) -> {
+ return unsuccessfulHandlerSuccessful;
+ });
}
@Test
public void hasNext()
{
HandlingInputRowIterator target =
createInputRowIterator(unsuccessfulHandler, unsuccessfulHandler);
Assert.assertTrue(target.hasNext());
- Assert.assertFalse(unsuccessfulHandler.invoked);
+ Mockito.verify(unsuccessfulHandler,
Mockito.never()).handle(any(InputRow.class));
}
@Test
public void yieldsNextIfUnhandled()
{
HandlingInputRowIterator target =
createInputRowIterator(unsuccessfulHandler, unsuccessfulHandler);
Assert.assertEquals(INPUT_ROW1, target.next());
- Assert.assertTrue(unsuccessfulHandler.invoked);
+ Mockito.verify(unsuccessfulHandler,
Mockito.atLeastOnce()).handle(any(InputRow.class));
Review comment:
I think we want to verify that `INPUT_ROW1` was passed to the handle
object
```suggestion
Mockito.verify(unsuccessfulHandler,
Mockito.atLeastOnce()).handle(INPUT_ROW1);
```
##########
File path:
core/src/test/java/org/apache/druid/data/input/HandlingInputRowIteratorTest.java
##########
@@ -81,58 +87,81 @@ public void throwsExceptionWhenYieldingNext()
public static class PresentRowTest
{
+ // Create variables for tracking behaviors of mock object
+
+ boolean unsuccessfulHandlerSuccessful;
+
+ // Create variables for tracking behaviors of mock object
+
+ boolean successfulHandlerSuccessful;
Review comment:
nit: These variables are not needed., instead just return `true` and
`false` on lines 115 and124 respectively.
##########
File path:
core/src/test/java/org/apache/druid/data/input/HandlingInputRowIteratorTest.java
##########
@@ -81,58 +87,81 @@ public void throwsExceptionWhenYieldingNext()
public static class PresentRowTest
{
+ // Create variables for tracking behaviors of mock object
+
+ boolean unsuccessfulHandlerSuccessful;
+
+ // Create variables for tracking behaviors of mock object
+
+ boolean successfulHandlerSuccessful;
+
private static final InputRow INPUT_ROW1 = EasyMock.mock(InputRow.class);
private static final InputRow INPUT_ROW2 = EasyMock.mock(InputRow.class);
private static final List<InputRow> INPUT_ROWS = Arrays.asList(INPUT_ROW1,
INPUT_ROW2);
- private TestInputRowHandler successfulHandler;
- private TestInputRowHandler unsuccessfulHandler;
+ private InputRowHandler successfulHandler;
+ private InputRowHandler unsuccessfulHandler;
@Before
public void setup()
{
- successfulHandler = new TestInputRowHandler(true);
- unsuccessfulHandler = new TestInputRowHandler(false);
+ // Construct mock object
+ successfulHandler = mock(HandlingInputRowIterator.InputRowHandler.class);
+ // Constructing variables that used for tracking behavior of the mocking
+ // object.
+ successfulHandlerSuccessful = true;
+ // Method Stubs
+
when(successfulHandler.handle(any(InputRow.class))).thenAnswer((stubInvo) -> {
+ return successfulHandlerSuccessful;
+ });
+ // Construct mock object
+ unsuccessfulHandler =
mock(HandlingInputRowIterator.InputRowHandler.class);
+ // Constructing variables that used for tracking behavior of the mocking
+ // object.
+ unsuccessfulHandlerSuccessful = false;
+ // Method Stubs
+
when(unsuccessfulHandler.handle(any(InputRow.class))).thenAnswer((stubInvo) -> {
+ return unsuccessfulHandlerSuccessful;
+ });
}
@Test
public void hasNext()
{
HandlingInputRowIterator target =
createInputRowIterator(unsuccessfulHandler, unsuccessfulHandler);
Assert.assertTrue(target.hasNext());
- Assert.assertFalse(unsuccessfulHandler.invoked);
+ Mockito.verify(unsuccessfulHandler,
Mockito.never()).handle(any(InputRow.class));
}
@Test
public void yieldsNextIfUnhandled()
{
HandlingInputRowIterator target =
createInputRowIterator(unsuccessfulHandler, unsuccessfulHandler);
Assert.assertEquals(INPUT_ROW1, target.next());
- Assert.assertTrue(unsuccessfulHandler.invoked);
+ Mockito.verify(unsuccessfulHandler,
Mockito.atLeastOnce()).handle(any(InputRow.class));
Review comment:
Why `atLeastOnce()` - is this really called multiple times? Similar
question for other uses of `atLeastOnce` in this test
##########
File path:
core/src/test/java/org/apache/druid/data/input/HandlingInputRowIteratorTest.java
##########
@@ -81,58 +87,81 @@ public void throwsExceptionWhenYieldingNext()
public static class PresentRowTest
{
+ // Create variables for tracking behaviors of mock object
+
+ boolean unsuccessfulHandlerSuccessful;
+
+ // Create variables for tracking behaviors of mock object
+
+ boolean successfulHandlerSuccessful;
+
private static final InputRow INPUT_ROW1 = EasyMock.mock(InputRow.class);
private static final InputRow INPUT_ROW2 = EasyMock.mock(InputRow.class);
Review comment:
Should we be using EasyMock and mockito in the same class? This seems
kinda confusing. My personal preference is mockito, but I'd be ok if you chose
to standardize on EasyMock too. As long as there is only one mocking system per
test class.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
