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: Combine sync_send_with_ping and sync_with_ping #28409

Merged
merged 1 commit into from Sep 6, 2023

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Sep 5, 2023

This reduces bloat, complexity, and makes tests less fragile to intermittent failures, see #27675 (comment).

This should not cause any noticeable slowdown, or may even be faster, because active polling will be done at most once.

@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 5, 2023

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK theStack
Concept ACK glozow, TheCharlatan

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Copy link
Member

@glozow glozow left a comment

Choose a reason for hiding this comment

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

Concept ACK fae0b21

Doesn't hurt to do it twice anyway. I think this is often what is intended.

@TheCharlatan
Copy link
Contributor

Concept ACK

Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

ACK fae0b21 🏓

@fanquake fanquake merged commit cf42182 into bitcoin:master Sep 6, 2023
15 checks passed
Frank-GER pushed a commit to syscoin/syscoin that referenced this pull request Sep 8, 2023
…ping

fae0b21 test: Combine sync_send_with_ping and sync_with_ping (MarcoFalke)

Pull request description:

  This reduces bloat, complexity, and makes tests less fragile to intermittent failures, see bitcoin#27675 (comment).

  This should not cause any noticeable slowdown, or may even be faster, because active polling will be done at most once.

ACKs for top commit:
  glozow:
    Concept ACK fae0b21
  theStack:
    ACK fae0b21 🏓

Tree-SHA512: 6c543241a7b85458dc7ff6a6203316b80a6227d83d38427e74f53f4c666a882647a8a221e5259071ee7bb5dfd63476fb03c9b558a1ea546734b14728c3c619ba
@maflcko maflcko deleted the 2309-test-ping- branch September 11, 2023 12:05
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Apr 15, 2024
Summary:
This reduces bloat, complexity, and makes tests less fragile to intermittent failures.

This is a backport of [[bitcoin/bitcoin#28409 | core#28409]]

Note: I don't know if this already fixes any existing intermittent failure, but at least it will prevent introducing a new one when backporting [[bitcoin/bitcoin#27675 | core#27675]]

Test Plan: `ninja check-functional`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D15979
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants