Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Unit tests of BlockBroadcaster #887

Merged
merged 35 commits into from
Feb 19, 2019

Conversation

smatthewenglish
Copy link
Contributor

@smatthewenglish smatthewenglish commented Feb 18, 2019

PR description

includes two unit tests:

--> one the confirms the propagateBlock method on EthPeer is invoked when all criteria are satisfied.

--> one that confirms the propagateBlock method on EthPeer is not invoked when all criteria are not satisfied.

NOTE
when 808 lands the change set of this code will be reduced to only the file BlockBroadcasterTest.java

Fixed Issue(s)

@@ -561,4 +566,25 @@ public void shouldNotImportBlocksThatAreAlreadyBeingImported() {

verify(ethScheduler, times(1)).scheduleSyncWorkerTask(any(Supplier.class));
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idea: Would it test any addition workflow (I'm not sure offhand) if you added a third node? ie, create the block on first node like currently. have node 2 process everything with native (non-test code), and wait for the message to arrive or process on 3rd node?

.mixHash(Hash.ZERO)
.nonce(0L)
.blockHashFunction(MainnetBlockHashFunction::createHash)
.buildBlockHeader();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just use BlockHeaderTestFixure to create a simple test block.

Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

}

@Test
public void blockPropagationRejectOnHasSeenBlock() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably don't need this one given we have the test above to check we send only to peers that haven't seen the block.

@smatthewenglish smatthewenglish merged commit ec8d821 into PegaSysEng:master Feb 19, 2019
smatthewenglish added a commit to smatthewenglish/pantheon that referenced this pull request Feb 19, 2019
rain-on pushed a commit to rain-on/pantheon that referenced this pull request Feb 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants