Skip to content

Comments

Add e2e tests for icm deploy and sendMsg commands#2798

Merged
artemanosov merged 12 commits intomainfrom
artem/add-e2e-tests-for-icm
May 20, 2025
Merged

Add e2e tests for icm deploy and sendMsg commands#2798
artemanosov merged 12 commits intomainfrom
artem/add-e2e-tests-for-icm

Conversation

@artemanosov
Copy link
Contributor

Why this should be merged

How this works

How this was tested

How is this documented

@artemanosov artemanosov changed the title Add e2e tests for icm deploy command Add e2e tests for icm deploy and sendMsg commands May 12, 2025
@artemanosov artemanosov force-pushed the artem/add-e2e-tests-for-icm branch from 385649a to 6cd0ef3 Compare May 12, 2025 21:26
@artemanosov artemanosov force-pushed the artem/add-e2e-tests-for-icm branch from 6cd0ef3 to d68bc95 Compare May 12, 2025 21:33
@sukantoraymond
Copy link
Contributor

Right now we have three commands that do the same thing:

  • interchain messenger
  • icm
  • teleporter

Can we just keep one and remove two?

@felipemadero

@felipemadero
Copy link
Collaborator

Right now we have three commands that do the same thing:

  • interchain messenger
  • icm
  • teleporter

Can we just keep one and remove two?

@felipemadero

i believe we can now remove teleporter. probably keep both interchain messenger and icm as an alias.
if the desire is to keep one, just icm ofc. I would only remove teleporter


output, err := utils.TestCommand(utils.ICMCmd, "deploy", commandArguments, globalFlags, testFlags)
gomega.Expect(err).Should(gomega.BeNil())
gomega.Expect(output).
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we want to trust CLI output? or instead try to find out at least that there are contracts at the expected locations?
not sure on this, just open question

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should find out if there are contract in that location -> also good knowledge for us to have anyways

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How would we do that?

Copy link
Contributor

Choose a reason for hiding this comment

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

@felipemadero are there functions we can implement to check if a contract exists in a blockchain?

Copy link
Collaborator

Choose a reason for hiding this comment

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

you can use ContractAlreadyDeployed from sdk/evm/evm.go

@artemanosov artemanosov force-pushed the artem/add-e2e-tests-for-icm branch from 4cec5e0 to 4f39539 Compare May 16, 2025 20:44
Copy link
Collaborator

@felipemadero felipemadero left a comment

Choose a reason for hiding this comment

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

LGTM. Much better if contracts are verified to be deployed directly on the L1. For messenger, it can also be checked that it is not deployed before

@artemanosov artemanosov merged commit 5d27b4d into main May 20, 2025
41 checks passed
@artemanosov artemanosov deleted the artem/add-e2e-tests-for-icm branch May 20, 2025 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants