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

metrics: tests: Fix flaky runtime_can_publish_metrics #7279

Conversation

alexggh
Copy link
Contributor

@alexggh alexggh commented May 24, 2023

When an re-org happens wait_for_blocks(2) would actually exit after the second import of blocks 1, so the conditions for the metric to exist won't be met hence the occasional test failure.

Fix that by adding a new methods which wait for blocks to be finalised, this should guarantee us that the preconditions for running the test are met.

More details in:
#7267

When an re-org happens wait_for_blocks(2) would actually exit after the second
import of blocks 1, so the conditions for the metric to exist won't be met
hence the occasional test failure.

More details in:
  paritytech#7267

Signed-off-by: Alexandru Gheorghe <[email protected]>
@alexggh alexggh added A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance. labels May 24, 2023
@alexggh alexggh requested review from sandreim and a user May 24, 2023 07:56
@alexggh alexggh self-assigned this May 24, 2023
@alexggh alexggh added the B0-silent Changes should not be mentioned in any release notes label May 24, 2023
Comment on lines 345 to 353
Box::pin(async move {
while let Some(notification) = import_notification_stream.next().await {
blocks.insert(notification.hash);
if blocks.len() == count {
break
}
}
})
.await;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Box::pin(async move {
while let Some(notification) = import_notification_stream.next().await {
blocks.insert(notification.hash);
if blocks.len() == count {
break
}
}
})
.await;
while let Some(notification) = import_notification_stream.next().await {
blocks.insert(notification.hash);
if blocks.len() == count {
break
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@alexggh
Copy link
Contributor Author

alexggh commented May 24, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit f654ffe into paritytech:master May 24, 2023
ordian added a commit that referenced this pull request May 25, 2023
* master:
  XCM: Tools for uniquely referencing messages (#7234)
  Companion: Substrate#13869 (#7119)
  Companion for Substrate#14214 (#7283)
  Fix flaky test and error reporting (#7282)
  impl guide: Update Collator Generation (#7250)
  Add staking-miner bin (#7273)
  metrics: tests: Fix flaky runtime_can_publish_metrics (#7279)
ordian added a commit that referenced this pull request May 25, 2023
…slashing-client

* ao-past-session-slashing-runtime:
  XCM: Tools for uniquely referencing messages (#7234)
  Companion: Substrate#13869 (#7119)
  Companion for Substrate#14214 (#7283)
  Fix flaky test and error reporting (#7282)
  impl guide: Update Collator Generation (#7250)
  Add staking-miner bin (#7273)
  metrics: tests: Fix flaky runtime_can_publish_metrics (#7279)
  [companion] Fix request-response protocols backpressure mechanism (#7276)
  PVF: instantiate runtime from bytes (#7270)
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. T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants