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

perf: speedup core smoke test #303

Merged
merged 8 commits into from
Jul 1, 2022
Merged

perf: speedup core smoke test #303

merged 8 commits into from
Jul 1, 2022

Conversation

strophy
Copy link
Collaborator

@strophy strophy commented Jun 17, 2022

Issue being fixed or feature implemented

Smoke tests were timing out without returning errors

What was done?

  • Move collect blockchaininfo call from before function to separate test
  • Refactor blockchaininfo call to timeout on individual node failure

How Has This Been Tested?

Locally and in CI running against test build here

Breaking Changes

None

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@strophy strophy added this to the v0.22.x milestone Jun 17, 2022
@strophy strophy requested a review from shumkov June 17, 2022 05:40
@strophy
Copy link
Collaborator Author

strophy commented Jun 28, 2022

@shumkov I've set dashevo/dashd-rpc#timeout as a dependency so we can see the dashd-rpc timeout feature changes from dashpay/dashd-rpc#45 and built a test image as strophy/dash-network-deploy:0.22.1-beta1. You can see this image running in CI here: https://github.com/dashevo/dash-network-ci/actions

The first batch of tests progresses much faster now, but I wonder if we should rewrite the masternode list check to either use the same design with promises, or query single nodes only until one of them responds, then use that response to check all nodes. What do you think?

@shumkov shumkov changed the title fix: smoke test timeout perf: speedup core smoke test Jul 1, 2022
Copy link
Member

@shumkov shumkov left a comment

Choose a reason for hiding this comment

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

Good job! 👍

@strophy strophy merged commit 0b13eca into master Jul 1, 2022
@strophy strophy deleted the fix/smoke-test-timeout branch July 1, 2022 11:19
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.

2 participants