IBFT message payload tests #404
Conversation
|
Collapsed the hierarchy in the message payloads to instead implement a Payload interface. |
| } | ||
|
|
||
| @Override | ||
| public String toString() { |
There was a problem hiding this comment.
what about the roundIdentifier stored in the baseclass?
There was a problem hiding this comment.
it's stored in the this class now as using an interface. But yes I forgot that.
Fixed.
| @@ -55,4 +57,30 @@ public void writeTo(final RLPOutput rlpOutput) { | |||
| public Collection<SignedData<PreparePayload>> getPreparePayloads() { | |||
| return preparePayloads; | |||
| } | |||
|
|
|||
| @Override | |||
| public boolean equals(final Object o) { | |||
There was a problem hiding this comment.
not sure we will ever do equality on these items in the real world code, could custom static functions be written in either test or testsupport.
There was a problem hiding this comment.
Could but I think it's cleaner from the consumers point of view to be able to rely on equals if needed.
| if (!super.equals(o)) { | ||
| return false; | ||
| } | ||
| final CommitPayload that = (CommitPayload) o; |
There was a problem hiding this comment.
has missed the RoundIdentifier, needed for the hashCode and toString (and I assume the other classes?)
There was a problem hiding this comment.
Just checked the other classes, looks like this was the unfortunate one that I forgot
| if (o == null || getClass() != o.getClass()) { | ||
| return false; | ||
| } | ||
| if (!super.equals(o)) { |
There was a problem hiding this comment.
can the super check be removed?
There was a problem hiding this comment.
That's autogenerated code. Yeah doesn't technically need it, but I'd rather just leave the intellij code as it
| Lists.newArrayList(), | ||
| Optional.empty(), | ||
| 0, | ||
| singletonList(Address.fromHexString(String.format("%020d", 1)))) |
There was a problem hiding this comment.
you can either:
AddressHelpers.ofValue(1), or
Address.fromhexStringLenient("1")
|
|
||
| final RLPInput rlpInput = RLP.input(rlpOut.encoded()); | ||
| final CommitPayload commitPayload = CommitPayload.readFrom(rlpInput); | ||
| assertThat(commitPayload.getRoundIdentifier()).isEqualTo(ROUND_IDENTIFIER); |
There was a problem hiding this comment.
nit: Do we want to check against "initial values" or just against "expectedCommitPayload"? If the latter, look at using "isEqualToComparingFieldByField".
(probably still need to do the MessageType")
I am assuming we trust that the constructor & getters do the right thing.
| @Test | ||
| public void roundTripRlp() { | ||
| final Hash digest = Hash.hash(BytesValue.of(1)); | ||
| final ConsensusRoundIdentifier expectedRoundIdentifier = new ConsensusRoundIdentifier(1, 1); |
There was a problem hiding this comment.
any reason this is written differently to the other tests? I.e. ROUND_IDENTIFIER being a class level variable?
|
|
||
| @Test | ||
| public void roundTripRlp() { | ||
| final Hash digest = Hash.hash(BytesValue.of(1)); |
There was a problem hiding this comment.
nit: diff to other test implementations
…e/pantheon into feature/ibft_message_payload_tests
PR description
IBFT message payload tests. These tests are focussed on round tripping the RLP encoding and decoding.
Fixed Issue(s)