Skip to content

Commit

Permalink
[TESTS] Clean up remove verification helpers
Browse files Browse the repository at this point in the history 
Now that there are cases where the SequenceNumberMap and Broadcast
are called, but no other internal state is updated, the existing helper
functions conflate too many decisions. Remove them in favor of explicitly
defining each state change expected.
  • Loading branch information
@julianknutsen
julianknutsen committed Nov 22, 2019
1 parent 0472ffc commit 6e2ea6e
Showing 1 changed file with 19 additions and 24 deletions.
43 changes: 19 additions & 24 deletions p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageProtectedStorageEntryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,15 +202,10 @@ void doProtectedStorageAddAndVerify(ProtectedStorageEntry protectedStorageEntry,

void doProtectedStorageRemoveAndVerify(ProtectedStorageEntry entry,
boolean expectedReturnValue,
boolean expectInternalStateChange) {

doProtectedStorageRemoveAndVerify(entry, expectedReturnValue, expectInternalStateChange, expectInternalStateChange);
}

void doProtectedStorageRemoveAndVerify(ProtectedStorageEntry entry,
boolean expectedReturnValue,
boolean expectInternalStateChange,
boolean expectSeqNrWrite) {
boolean expectedHashMapAndDataStoreUpdated,
boolean expectedListenersSignaled,
boolean expectedBroadcast,
boolean expectedSeqNrWrite) {

SavedTestState beforeState = this.testState.saveTestState(entry);

Expand All @@ -219,7 +214,7 @@ void doProtectedStorageRemoveAndVerify(ProtectedStorageEntry entry,
if (!this.useMessageHandler)
Assert.assertEquals(expectedReturnValue, addResult);

this.testState.verifyProtectedStorageRemove(beforeState, entry, expectInternalStateChange, expectInternalStateChange, expectSeqNrWrite, expectSeqNrWrite, this.expectIsDataOwner());
this.testState.verifyProtectedStorageRemove(beforeState, entry, expectedHashMapAndDataStoreUpdated, expectedListenersSignaled, expectedBroadcast, expectedSeqNrWrite, this.expectIsDataOwner());
}

/// Valid Add Tests (isValidForAdd() and matchesRelevantPubKey() return true)
Expand Down Expand Up @@ -272,7 +267,7 @@ public void addProtectectedStorageEntry_afterRemoveSameSeqNr() {
ProtectedStorageEntry entryForRemove = this.getProtectedStorageEntryForRemove(1);

doProtectedStorageAddAndVerify(entryForAdd, true, true);
doProtectedStorageRemoveAndVerify(entryForRemove, false, false);
doProtectedStorageRemoveAndVerify(entryForRemove, false, false, false, false, false);

doProtectedStorageAddAndVerify(entryForAdd, false, false);
}
Expand Down Expand Up @@ -320,7 +315,7 @@ public void remove_seqNrEqAddSeqNr() {

doProtectedStorageAddAndVerify(entryForAdd, true, true);

doProtectedStorageRemoveAndVerify(entryForRemove, false, false);
doProtectedStorageRemoveAndVerify(entryForRemove, false, false, false, false, false);
}

// TESTCASE: Removing an item after successfully added (remove seq # > add seq #)
Expand All @@ -330,15 +325,15 @@ public void remove_seqNrGtAddSeqNr() {
ProtectedStorageEntry entryForRemove = this.getProtectedStorageEntryForRemove(2);

doProtectedStorageAddAndVerify(entryForAdd, true, true);
doProtectedStorageRemoveAndVerify(entryForRemove, true, true);
doProtectedStorageRemoveAndVerify(entryForRemove, true, true, true, true, true);
}

// TESTCASE: Removing an item before it was added. This triggers a SequenceNumberMap write, but nothing else
// TESTCASE: Removing an item before it was added. This triggers a SequenceNumberMap write and broadcast
@Test
public void remove_notExists() {
ProtectedStorageEntry entryForRemove = this.getProtectedStorageEntryForRemove(1);

doProtectedStorageRemoveAndVerify(entryForRemove, true, false, true);
doProtectedStorageRemoveAndVerify(entryForRemove, true, false, false, true, true);
}

// TESTCASE: Removing an item after successfully adding (remove seq # < add seq #)
Expand All @@ -348,7 +343,7 @@ public void remove_seqNrLessAddSeqNr() {
ProtectedStorageEntry entryForRemove = this.getProtectedStorageEntryForRemove(1);

doProtectedStorageAddAndVerify(entryForAdd, true, true);
doProtectedStorageRemoveAndVerify(entryForRemove, false, false);
doProtectedStorageRemoveAndVerify(entryForRemove, false, false, false, false, false);
}

// TESTCASE: Add after removed (same seq #)
Expand All @@ -358,7 +353,7 @@ public void add_afterRemoveSameSeqNr() {
doProtectedStorageAddAndVerify(entryForAdd, true, true);

ProtectedStorageEntry entryForRemove = this.getProtectedStorageEntryForRemove(2);
doProtectedStorageRemoveAndVerify(entryForRemove, true, true);
doProtectedStorageRemoveAndVerify(entryForRemove, true, true, true, true, true);

doProtectedStorageAddAndVerify(entryForAdd, false, false);
}
Expand All @@ -370,7 +365,7 @@ public void add_afterRemoveGreaterSeqNr() {
doProtectedStorageAddAndVerify(entryForAdd, true, true);

ProtectedStorageEntry entryForRemove = this.getProtectedStorageEntryForRemove(2);
doProtectedStorageRemoveAndVerify(entryForRemove, true, true);
doProtectedStorageRemoveAndVerify(entryForRemove, true, true, true, true, true);

entryForAdd = this.getProtectedStorageEntryForAdd(3);
doProtectedStorageAddAndVerify(entryForAdd, true, true);
Expand All @@ -385,7 +380,7 @@ public void remove_EntryNotisValidForRemoveOperation() {
doProtectedStorageAddAndVerify(entryForAdd, true, true);

ProtectedStorageEntry entryForRemove = this.getProtectedStorageEntryForRemove(2, false, true);
doProtectedStorageRemoveAndVerify(entryForRemove, false, false);
doProtectedStorageRemoveAndVerify(entryForRemove, false, false, false, false, false);
}

// TESTCASE: Remove fails if Entry is valid for remove, but metadata doesn't match remove target
Expand All @@ -395,7 +390,7 @@ public void remove_EntryNotmatchesRelevantPubKey() {
doProtectedStorageAddAndVerify(entryForAdd, true, true);

ProtectedStorageEntry entryForRemove = this.getProtectedStorageEntryForRemove(2, true, false);
doProtectedStorageRemoveAndVerify(entryForRemove, false, false);
doProtectedStorageRemoveAndVerify(entryForRemove, false, false, false, false, false);
}

// TESTCASE: Remove fails if Entry is not valid for remove and metadata doesn't match remove target
Expand All @@ -405,7 +400,7 @@ public void remove_EntryNotisValidForRemoveOperationNotmatchesRelevantPubKey() {
doProtectedStorageAddAndVerify(entryForAdd, true, true);

ProtectedStorageEntry entryForRemove = this.getProtectedStorageEntryForRemove(2, false, false);
doProtectedStorageRemoveAndVerify(entryForRemove, false, false);
doProtectedStorageRemoveAndVerify(entryForRemove, false, false, false, false, false);
}


Expand All @@ -416,7 +411,7 @@ public void add_afterRemoveLessSeqNr() {
doProtectedStorageAddAndVerify(entryForAdd, true, true);

ProtectedStorageEntry entryForRemove = this.getProtectedStorageEntryForRemove(3);
doProtectedStorageRemoveAndVerify(entryForRemove, true, true);
doProtectedStorageRemoveAndVerify(entryForRemove, true, true, true, true, true);

entryForAdd = this.getProtectedStorageEntryForAdd(1);
doProtectedStorageAddAndVerify(entryForAdd, false, false);
Expand Down Expand Up @@ -567,7 +562,7 @@ public void refreshTTL_refreshAfterRemove() throws CryptoException {
ProtectedStorageEntry entryForRemove = this.getProtectedStorageEntryForRemove(2);

doProtectedStorageAddAndVerify(entryForAdd, true, true);
doProtectedStorageRemoveAndVerify(entryForRemove, true, true);
doProtectedStorageRemoveAndVerify(entryForRemove, true, true, true, true, true);

doRefreshTTLAndVerify(buildRefreshOfferMessage(entryForAdd, this.payloadOwnerKeys,3), false, false);
}
Expand Down Expand Up @@ -664,7 +659,7 @@ public void add_afterRemoveGreaterSeqNr() {
doProtectedStorageAddAndVerify(entryForAdd, true, true);

ProtectedStorageEntry entryForRemove = this.getProtectedStorageEntryForRemove(2);
doProtectedStorageRemoveAndVerify(entryForRemove, true, true);
doProtectedStorageRemoveAndVerify(entryForRemove, true, true, true, true, true);

entryForAdd = this.getProtectedStorageEntryForAdd(3);
doProtectedStorageAddAndVerify(entryForAdd, false, false);
Expand Down

0 comments on commit 6e2ea6e

Please sign in to comment.