[TESTS] Clean up mockito never() and eq(null) usages

never() and any() don't play well together for nullable types. Change
the broadcast mocks to user nullable() and fixup tests.
This commit is contained in:
Julian Knutsen 2019-11-22 17:51:35 -08:00
parent 1bd450b3bc
commit 17f4b7096e
No known key found for this signature in database
GPG Key ID: D85F536DB3615B2D
3 changed files with 23 additions and 22 deletions

View File

@ -188,7 +188,7 @@ public class P2PDataStorageClientAPITest {
SavedTestState beforeState = this.testState.saveTestState(protectedMailboxStorageEntry);
Assert.assertTrue(this.testState.mockedStorage.remove(protectedMailboxStorageEntry, TestState.getTestNodeAddress()));
this.testState.verifyProtectedStorageRemove(beforeState, protectedMailboxStorageEntry, false, false, false, true);
this.testState.verifyProtectedStorageRemove(beforeState, protectedMailboxStorageEntry, false, false, true, true);
}
// TESTCASE: Adding, then removing a mailbox message from the getMailboxDataWithSignedSeqNr API

View File

@ -76,7 +76,11 @@ public class P2PDataStoragePersistableNetworkPayloadTest {
ON_MESSAGE,
}
void doAddAndVerify(PersistableNetworkPayload persistableNetworkPayload, boolean expectedReturnValue, boolean expectedStateChange) {
void doAddAndVerify(PersistableNetworkPayload persistableNetworkPayload,
boolean expectedReturnValue,
boolean expectedHashMapAndDataStoreUpdated,
boolean expectedListenersSignaled,
boolean expectedBroadcast) {
SavedTestState beforeState = this.testState.saveTestState(persistableNetworkPayload);
if (this.testCase == TestCase.PUBLIC_API) {
@ -89,7 +93,7 @@ public class P2PDataStoragePersistableNetworkPayloadTest {
testState.mockedStorage.onMessage(new AddPersistableNetworkPayloadMessage(persistableNetworkPayload), mockedConnection);
}
this.testState.verifyPersistableAdd(beforeState, persistableNetworkPayload, expectedStateChange, expectedStateChange, expectedStateChange);
this.testState.verifyPersistableAdd(beforeState, persistableNetworkPayload, expectedHashMapAndDataStoreUpdated, expectedListenersSignaled, expectedBroadcast);
}
@Before
@ -119,16 +123,15 @@ public class P2PDataStoragePersistableNetworkPayloadTest {
@Test
public void addPersistableNetworkPayload() {
// First add should succeed regardless of parameters
doAddAndVerify(this.persistableNetworkPayload, true, true);
doAddAndVerify(this.persistableNetworkPayload, true, true, true, true);
}
@Test
public void addPersistableNetworkPayloadDuplicate() {
doAddAndVerify(this.persistableNetworkPayload, true, true);
doAddAndVerify(this.persistableNetworkPayload, true, true, true, true);
// Second call only succeeds if reBroadcast was set
boolean expectedReturnValue = this.reBroadcast;
doAddAndVerify(this.persistableNetworkPayload, expectedReturnValue, false);
// We return true and broadcast if reBroadcast is set
doAddAndVerify(this.persistableNetworkPayload, this.reBroadcast, false, false, this.reBroadcast);
}
}
@ -145,7 +148,7 @@ public class P2PDataStoragePersistableNetworkPayloadTest {
public void invalidHash() {
PersistableNetworkPayload persistableNetworkPayload = new PersistableNetworkPayloadStub(false);
doAddAndVerify(persistableNetworkPayload, false, false);
doAddAndVerify(persistableNetworkPayload, false, false, false, false);
}
}
@ -168,7 +171,7 @@ public class P2PDataStoragePersistableNetworkPayloadTest {
// The onMessage path checks for tolerance
boolean expectedReturn = this.testCase != TestCase.ON_MESSAGE;
doAddAndVerify(persistableNetworkPayload, expectedReturn, expectedReturn);
doAddAndVerify(persistableNetworkPayload, expectedReturn, expectedReturn, expectedReturn, expectedReturn);
}
}
}

View File

@ -187,10 +187,10 @@ public class TestState {
verify(this.appendOnlyDataStoreListener, never()).onAdded(persistableNetworkPayload);
if (expectedBroadcast)
verify(this.mockBroadcaster).broadcast(any(AddPersistableNetworkPayloadMessage.class), any(NodeAddress.class),
eq(null));
verify(this.mockBroadcaster).broadcast(any(AddPersistableNetworkPayloadMessage.class),
nullable(NodeAddress.class), isNull());
else
verify(this.mockBroadcaster, never()).broadcast(any(BroadcastMessage.class), any(NodeAddress.class), any(BroadcastHandler.Listener.class));
verify(this.mockBroadcaster, never()).broadcast(any(BroadcastMessage.class), nullable(NodeAddress.class), nullable(BroadcastHandler.Listener.class));
}
void verifyProtectedStorageAdd(SavedTestState beforeState,
@ -219,14 +219,13 @@ public class TestState {
if (expectedBroadcast) {
final ArgumentCaptor<BroadcastMessage> captor = ArgumentCaptor.forClass(BroadcastMessage.class);
verify(this.mockBroadcaster).broadcast(captor.capture(), any(NodeAddress.class),
eq(null));
verify(this.mockBroadcaster).broadcast(captor.capture(), nullable(NodeAddress.class), isNull());
BroadcastMessage broadcastMessage = captor.getValue();
Assert.assertTrue(broadcastMessage instanceof AddDataMessage);
Assert.assertEquals(protectedStorageEntry, ((AddDataMessage) broadcastMessage).getProtectedStorageEntry());
} else {
verify(this.mockBroadcaster, never()).broadcast(any(BroadcastMessage.class), any(NodeAddress.class), any(BroadcastHandler.Listener.class));
verify(this.mockBroadcaster, never()).broadcast(any(BroadcastMessage.class), nullable(NodeAddress.class), nullable(BroadcastHandler.Listener.class));
}
if (expectedSequenceNrMapWrite) {
@ -276,7 +275,7 @@ public class TestState {
verify(this.mockSeqNrStorage, never()).queueUpForSave(any(SequenceNumberMap.class), anyLong());
if (!expectedBroadcast)
verify(this.mockBroadcaster, never()).broadcast(any(BroadcastMessage.class), any(NodeAddress.class), any(BroadcastHandler.Listener.class));
verify(this.mockBroadcaster, never()).broadcast(any(BroadcastMessage.class), nullable(NodeAddress.class), nullable(BroadcastHandler.Listener.class));
protectedStorageEntries.forEach(protectedStorageEntry -> {
@ -288,9 +287,9 @@ public class TestState {
if (expectedBroadcast) {
if (protectedStorageEntry instanceof ProtectedMailboxStorageEntry)
verify(this.mockBroadcaster).broadcast(any(RemoveMailboxDataMessage.class), any(NodeAddress.class), eq(null));
verify(this.mockBroadcaster).broadcast(any(RemoveMailboxDataMessage.class), nullable(NodeAddress.class), isNull());
else
verify(this.mockBroadcaster).broadcast(any(RemoveDataMessage.class), any(NodeAddress.class), eq(null));
verify(this.mockBroadcaster).broadcast(any(RemoveDataMessage.class), nullable(NodeAddress.class), isNull());
}
@ -320,8 +319,7 @@ public class TestState {
Assert.assertTrue(entryAfterRefresh.getCreationTimeStamp() > beforeState.creationTimestampBeforeUpdate);
final ArgumentCaptor<BroadcastMessage> captor = ArgumentCaptor.forClass(BroadcastMessage.class);
verify(this.mockBroadcaster).broadcast(captor.capture(), any(NodeAddress.class),
eq(null));
verify(this.mockBroadcaster).broadcast(captor.capture(), nullable(NodeAddress.class), isNull());
BroadcastMessage broadcastMessage = captor.getValue();
Assert.assertTrue(broadcastMessage instanceof RefreshOfferMessage);
@ -338,7 +336,7 @@ public class TestState {
Assert.assertEquals(beforeState.creationTimestampBeforeUpdate, entryAfterRefresh.getCreationTimeStamp());
}
verify(this.mockBroadcaster, never()).broadcast(any(BroadcastMessage.class), any(NodeAddress.class), any(BroadcastHandler.Listener.class));
verify(this.mockBroadcaster, never()).broadcast(any(BroadcastMessage.class), nullable(NodeAddress.class), nullable(BroadcastHandler.Listener.class));
verify(this.mockSeqNrStorage, never()).queueUpForSave(any(SequenceNumberMap.class), anyLong());
}
}