Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Less sleeps #9848

Merged
11 commits merged into from
Sep 29, 2021
Merged

Less sleeps #9848

11 commits merged into from
Sep 29, 2021

Conversation

gilescope
Copy link
Contributor

@gilescope gilescope commented Sep 23, 2021

Rather than sleeping in tests, waiting for blocks to go by, we check over RPC for a few blocks to be produced.
( Fixes: #9773 )
I thought that the tests would probably be more reliable if we polled rather than streamed the chain head.

(couple of drive by typos & clippies fixed also)

Dependencies:

wait-timeout = "0.2" is already a dependency of assert_cmd which we use.
tokio and jsonrpsee-ws-client are used already with those features.

@gilescope gilescope added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Sep 23, 2021
@dvdplm
Copy link
Contributor

dvdplm commented Sep 24, 2021

What is the impact of this on total time to run the test suite?

bin/node/cli/Cargo.toml Outdated Show resolved Hide resolved
bin/node/cli/tests/common.rs Outdated Show resolved Hide resolved
bin/node/cli/tests/common.rs Outdated Show resolved Hide resolved
bin/node/cli/tests/common.rs Outdated Show resolved Hide resolved
bin/node/cli/tests/common.rs Outdated Show resolved Hide resolved
@gilescope
Copy link
Contributor Author

@dvdplm on build timings: the existing linux stable build takes anywhere from 35 to 50 mins. This one took 35min 2 seconds so maybe we're close to breaking the 35 min barrier.

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Besides last nitpick, it looks good.

bin/node/cli/tests/temp_base_path_works.rs Outdated Show resolved Hide resolved
bin/node/cli/tests/temp_base_path_works.rs Outdated Show resolved Hide resolved
gilescope and others added 2 commits September 29, 2021 10:35
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
@bkchr
Copy link
Member

bkchr commented Sep 29, 2021

bot merge

@ghost
Copy link

ghost commented Sep 29, 2021

Trying merge.

@ghost
Copy link

ghost commented Sep 29, 2021

Bot will approve on the behalf of @bkchr, since they are a team lead, in an attempt to reach the minimum approval count

@ghost ghost merged commit 0a785f1 into paritytech:master Sep 29, 2021
ordian added a commit that referenced this pull request Oct 2, 2021
* master: (67 commits)
  Downstream `node-template` pull (#9915)
  Implement core::fmt::Debug for BoundedVec (#9914)
  Quickly skip invalid transactions during block authorship. (#9789)
  Add SS58 prefix for Automata (#9805)
  Clean up sc-peerset (#9806)
  Test each benchmark case in own #[test] (#9860)
  Add build with docker section to README (#9792)
  Simple Trait to Inspect Metadata (#9893)
  Pallet Assets: Create new asset classes from genesis config (#9742)
  doc: subkey usage (#9905)
  Silence alert about large-statement-fetcher (#9882)
  Fix democracy on-initialize weight (#9890)
  Fix basic authorship flaky test (#9906)
  contracts: Add event field names (#9896)
  subkey readme update on install (#9900)
  add feature wasmtime-jitdump (#9871)
  Return `target_hash` for finality_target instead of an Option (#9867)
  Update wasmtime to 0.29.0 (#9552)
  Less sleeps (#9848)
  remove unidiomatic (#9895)
  ...
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor node-cli "integration tests"
3 participants