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

test(docker): address FIXMEs #675

Merged
merged 7 commits into from
Apr 3, 2023
Merged

Conversation

tbruyelle
Copy link
Contributor

@tbruyelle tbruyelle commented Mar 29, 2023

This change is originated by #495, in order to add an integration test that broadcast TXs.

The following cases have been added to docker integration tests:

  • improve auth/account assertions
  • add a package
  • broadcast a TX

Note that to broadcast a tx I had to add the test1 account key in the running container, thanks to the seed.

Also address others FIXMEs:

  • waitGnoland(): improve by looking at the logs instead of waiting 20s
  • checkDocker(): implement by looking at docker info output
  • buildDockerImage(): remove output check, assuming that if docker build fails, the command will return an error
  • cleanupGnoland(): use only docker rm -f because it also kills the container if it's running.

@tbruyelle tbruyelle requested a review from a team as a code owner March 29, 2023 21:29
@tbruyelle tbruyelle changed the title test(docker): address FIXME test(docker): address FIXMEs Mar 30, 2023
@moul
Copy link
Member

moul commented Mar 31, 2023

CI output for the curious:

CleanShot 2023-03-31 at 18 48 34@2x

Copy link
Member

@moul moul left a comment

Choose a reason for hiding this comment

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

LGTM, can you just wait for #585 to be merged before, please?

@tbruyelle
Copy link
Contributor Author

LGTM, can you just wait for #585 to be merged before, please?

Sure that was my intention. Same for my other PRs.

@moul
Copy link
Member

moul commented Apr 3, 2023

@tbruyelle can you rebase this PR with master?

@tbruyelle
Copy link
Contributor Author

@tbruyelle can you rebase this PR with master?

Done!

@moul moul merged commit aa730e3 into gnolang:master Apr 3, 2023
4 checks passed
grepsuzette pushed a commit to grepsuzette/gno that referenced this pull request Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants