You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/08/04 01:07:53 UTC
[GitHub] [druid] suneet-s commented on a change in pull request #11529: Replace TestInputRowHandler with mocking object
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org