-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
deployment/ccip/changeset: add messaging test #15166
Conversation
Small refactor to the request helpers to allow for specifying all fields of the message.
I see you updated files related to
|
AER Report: Operator UI CI ran successfully ✅AER Report: CI Core ran successfully ✅ |
@@ -83,7 +84,13 @@ func TestInitialDeployOnLocal(t *testing.T) { | |||
require.NoError(t, err) | |||
block := latesthdr.Number.Uint64() | |||
startBlocks[dest] = &block | |||
seqNum := ccdeploy.TestSendRequest(t, e, state, src, dest, false, nil) | |||
seqNum := ccdeploy.TestSendRequest(t, e, state, src, dest, false, router.ClientEVM2AnyMessage{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit/ what about a TestSend(Dummy|Default)Request
? I could convenient to have it so that we won't deal with creating those fields if we are not going to use them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I thought about adding that but I think we wanna see how the usage of these funcs evolves and build abstractions after we have many use cases. I think for now this is the most flexible, and can improve further.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
ffd593f
Also: Small refactor to the request helpers to allow for specifying all fields of the message.
Requires smartcontractkit/chainlink-ccip#311