Commit plugin e2e tests using OCR3Runner #82
Conversation
| @@ -106,6 +106,8 @@ func (o ObserverImpl) ObserveOffRampNextSeqNums(ctx context.Context) []plugintyp | |||
| o.lggr.Warnw("call to KnownSourceChainsSlice failed", "err", err) | |||
| return nil | |||
| } | |||
|
|
|||
| sort.Slice(sourceChains, func(i, j int) bool { return sourceChains[i] < sourceChains[j] }) | |||
There was a problem hiding this comment.
Is this sort necessary here? I thought in the aggregation step we would handle different orders somehow
There was a problem hiding this comment.
Makes testing much easier, because we can verify what's passed in the call in a certain order and match specific cases with return data
…nlink-ccip into dk/commit-e2e-test
commit/plugin_e2e_test.go
Outdated
| nodeIDs := []commontypes.OracleID{1, 2, 3} | ||
| peerIDs := []libocrtypes.PeerID{{1}, {2}, {3}} | ||
|
|
||
| oracleIDToP2pID := make(map[commontypes.OracleID]libocrtypes.PeerID) |
There was a problem hiding this comment.
nit: maybe we can move this and chainConfig definitions to a setup func that returns all of this data in a struct?
There was a problem hiding this comment.
Made many changes to the test since u reviewed, lmk if you still have the same suggestion
commit/plugin_e2e_test.go
Outdated
| require.Len(t, mapset.NewSet(destChain, sourceChain1, sourceChain2).ToSlice(), 3) | ||
| require.Less(t, sourceChain1, sourceChain2, "test below expected source chain1 to be LT chain2") |
There was a problem hiding this comment.
i have it just in case someone updates the test
commit/plugin_e2e_test.go
Outdated
|
|
||
| func TestPlugin_E2E_AllNodesAgree(t *testing.T) { | ||
| require.Len(t, mapset.NewSet(destChain, sourceChain1, sourceChain2).ToSlice(), 3) | ||
| require.Less(t, sourceChain1, sourceChain2, "test below expected source chain1 to be LT chain2") |
There was a problem hiding this comment.
Is this here because the arrays / etc. will be sorted a certain way by the plugin?
There was a problem hiding this comment.
yeah, and that's how this test has been set up
commit/plugin_e2e_test.go
Outdated
| ) | ||
|
|
||
| func TestPlugin_E2E_AllNodesAgree(t *testing.T) { | ||
| require.Len(t, mapset.NewSet(destChain, sourceChain1, sourceChain2).ToSlice(), 3) |
There was a problem hiding this comment.
This check seems kind of weird, maybe we can pull out mapset.NewSet(destChain, sourceChain1, sourceChain2).ToSlice() into a var and then here we do a require.Len(t, chains, 3) if the goal is to make sure we don't add chains to the test?
|
I removed both of this test setup validations, since they seem confusing. Test will just fail in case someone edits the test setup, but shouldn't be hard to figure out why. |
Test Coverage
|
OCR3Runner commit plugin e2e unit tests.
Plugin still has lot's of tests missing, with this PR we achieve
+20%coverage in our codebase and few bugs were already found and fixed. I don't want to make this PR huge, will add more tests in separate ones. Thank you.