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.

Sign up for GitHub

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

Jump to bottom

chore[e2e]: parse from result #5965

Merged
merged 16 commits into from
Aug 7, 2023
Merged

chore[e2e]: parse from result #5965

merged 16 commits into from
Aug 7, 2023

Conversation

czarcas7ic
Copy link
Member

@czarcas7ic czarcas7ic commented Aug 5, 2023 •
edited
Loading

Closes: #XXX

What is the purpose of the change

Various improvements to the e2e test suite, most importantly improving flakiness:

  • Removes "latest" tracker (latestLockNumber, latestProposalNumber, etc) in favor of parsing from result
  • Spreads tests across both chains across all nodes, instead of running 95 percent of tests on ChainA's node0
  • Explicitly tells hermes to clear packets after an ibc tx is sent
  • Mutex on sender and reciever address for IBC send method.
    • This prevents issues when querying balance on receiving chain when multiple tests are running
  • Speeds up epoch to 5s from 60s
  • Various clean up of repetitive methods

Testing and Verifying

Has run 20 times in a row now with no errors. Usually 1 in every 7 fail.

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes?
  • Changelog entry added to Unreleased section of CHANGELOG.md?

Where is the change documented?

  • Specification (x/{module}/README.md)
  • Osmosis documentation site
  • Code comments?
  • N/A

@czarcas7ic czarcas7ic added V:state/compatible/no_backport State machine compatible PR, depends on prior breaks A:no-changelog labels Aug 5, 2023
@czarcas7ic czarcas7ic marked this pull request as ready for review August 5, 2023 04:27
@czarcas7ic czarcas7ic marked this pull request as draft August 5, 2023 05:01
@github-actions github-actions bot removed the T:build label Aug 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2023

Important Notice

This PR includes modifications to the tests/e2e/initialization module.
Please follow the instructions below:

  1. Backport these changes to the previous Osmosis version's branch.
  2. Run the script inside a Docker container to update genesis and configs for pre-upgrade Osmosis.
  3. Merge the backported changes.
  4. The image will be built and uploaded to Docker Hub here.
  5. Grab the latest image and update it in the PR to the main branch replacing the previousVersionInitTag in the osmosis/tests/e2e/containers/config.go

Please let us know if you need any help.

@czarcas7ic czarcas7ic marked this pull request as ready for review August 7, 2023 19:15
ValarDragon
ValarDragon approved these changes Aug 7, 2023
View reviewed changes
Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

confirmed E2E only, just merging based on request

@ValarDragon ValarDragon merged commit 03d558b into main Aug 7, 2023
@ValarDragon ValarDragon deleted the adam/parse-from-result-e2e branch August 7, 2023 20:00
p0mvn pushed a commit that referenced this pull request Aug 29, 2023
@czarcas7ic @p0mvn
chore[e2e]: parse from result (#5965)
369f9a5 
* parse from result e2e

* no err check for os.Remove

* default node

* more logging

* fix

* add mutex for IBC sends

* clean up

* add node level mutex

* add wallet address across all validators

* use any available node (chainA or chainB)

* properly release nodes

* mutex for addr rather than entire chain

* faster epoch

* remove use of public address

* potential revert, remove node mutex

* full remove node level mutex
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ValarDragon ValarDragon ValarDragon approved these changes

Assignees
No one assigned
Labels
A:no-changelog V:state/compatible/no_backport State machine compatible PR, depends on prior breaks
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@czarcas7ic @ValarDragon