Skip to content
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

Implement relayer integration tests #395

Merged
merged 15 commits into from
Feb 22, 2021
Merged

Implement relayer integration tests #395

merged 15 commits into from
Feb 22, 2021

Conversation

orkunkl
Copy link
Contributor

@orkunkl orkunkl commented Jan 28, 2021

This PR basically installs relayer on CI level, then executes the bash tests which located at /integration. These setup scripts are copied from relayer tests. For now the integration test setups up two wasm chains and sends token from one to another, then checks the balance. I made some changes on CI build. Not sure it will work as intended in the first try. Open for suggestions.

@orkunkl orkunkl requested review from alpe and ethanfrey January 28, 2021 15:15
@ethanfrey
Copy link
Member

Hmmm... this assumes the relayer works?

If it were stable, I think this would be a very good test. As relayer is still somewhat buggy and not at 1.0, I would wait. This branch is a good start, but let's make sure the other side is stable before we test our code against it.

@orkunkl
Copy link
Contributor Author

orkunkl commented Jan 28, 2021

@ethanfrey Actually relayer works with recently launched two local networks. So tests will pass

@alpe
Copy link
Contributor

alpe commented Jan 29, 2021

I appreciate the effort ❤️ but I have mixed feelings about adding the relayer into automated CI tests.
One thing is adding and relying on a new dependency but under the hood what is it really testing that we have not covered by integration tests in our project or cosmos-sdk?
It is definitively a smoke test on the compiled binary but mostly the relayer I afraid!
But before working with 2 chains we should have automated system tests for a single instance covering core wasmd functionality. There are some scripts in contrib/local for example addressing basic code upload/instantiation/execution/ queries...

How to move forward? The scripts are adding value to the project and make it easier to get started for anybody testing out IBC. Running them on CI is a bit controversy. I would suggest to move them to contrib/relayer-demo or a similar path. WDYT?

@orkunkl
Copy link
Contributor Author

orkunkl commented Feb 1, 2021

@alpe I agree these tests are bit shallow and insufficient. But at some point we will need IBC integration tests. We can leave the branch as it is, or exclude the tests from CI.

@orkunkl
Copy link
Contributor Author

orkunkl commented Feb 3, 2021

Moved tests to contrib

Copy link
Contributor

@alpe alpe left a comment

Choose a reason for hiding this comment

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

Looks good.:tulip:
A nice to have would be a README in the relayer-tests folder with thank you to the original authors and link to their repo.

@orkunkl orkunkl merged commit 14b4d16 into master Feb 22, 2021
@orkunkl orkunkl deleted the relayer-it-tests branch February 22, 2021 10:26
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