Test - Validator #340
Test - Validator #340
Conversation
| "github.com/bloxapp/ssv-spec/types/testingutils" | ||
| ) | ||
|
|
||
| func ValidConsensus() tests.SpecTest { |
ssv/spectest/tests/validator/test.go
Outdated
| } | ||
|
|
||
| func (test *ValidatorTest) GetPostState() (interface{}, error) { | ||
| return nil, nil |
There was a problem hiding this comment.
as part of the post-mortem about the deneb bug, we concluded that we should generate jsons for all tests..
so might as well implement this
There was a problem hiding this comment.
Adding. Thanks!
ssv/spectest/tests/validator/test.go
Outdated
| for _, msg := range test.OutputMessages { | ||
| found := false | ||
| for index2, msg2 := range broadcastedMsgs { | ||
| if usedIndexes[index2] { | ||
| continue | ||
| } | ||
| if bytes.Equal(msg.GetData(), msg2.GetData()) { | ||
| if msg.GetType() == msg2.GetType() && msg.GetID() == msg2.GetID() { | ||
| found = true | ||
| usedIndexes[index2] = true | ||
| break | ||
| } | ||
| } | ||
| } | ||
| if !found { | ||
| panic(fmt.Sprintf("Broadcasted message not found in output messages: %v", msg)) | ||
| } |
There was a problem hiding this comment.
This checks that OutputMessages are a subset of broadcastedMsgs but not the other way around?
meaning that could be a BroadcastedMsg that is missing in OutputMessage?
I think maybe more tests are like this? I just never really inspected the code
There was a problem hiding this comment.
Before it I do:
require.Len(t, broadcastedMsgs, len(test.OutputMessages))So I think they are tested to be equal. What do you think?
ssv/spectest/tests/validator/test.go
Outdated
| require.Len(t, broadcastedRoots, len(test.BeaconBroadcastedRoots)) | ||
| for _, r1 := range test.BeaconBroadcastedRoots { | ||
| found := false | ||
| for _, r2 := range broadcastedRoots { | ||
| if r1 == hex.EncodeToString(r2[:]) { | ||
| found = true | ||
| break | ||
| } | ||
| } | ||
| require.Truef(t, found, "broadcasted beacon root not found") | ||
| } |
There was a problem hiding this comment.
same issue as below I guess
There was a problem hiding this comment.
Before it I also check the length. But indeed I should add an "already used" map as in the other check. Adding it.
| Duties []*types.Duty | ||
| Messages []*types.SSVMessage | ||
| OutputMessages []*types.SSVMessage | ||
| BeaconBroadcastedRoots []string |
There was a problem hiding this comment.
I noticed you don't really use this field for the tests...
Not much harm that it is here I suppose
your choice whether to remove it or not
There was a problem hiding this comment.
Which field?
There was a problem hiding this comment.
BeaconBroadcastedRoots
There was a problem hiding this comment.
I use it in compareBroadcastedBeaconMsgs.
There was a problem hiding this comment.
it is just that in the tests themselves I noticed it was always empty
but arguably that's also a check
There was a problem hiding this comment.
Oh, I see. Will take a look. Thx!
|
@GalRogozinski |
SSV Tests
Added tests for the
Validatorstructure.Tests:
Solving one element of issue 25.