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: Fail if connect_nodes fails #25443

Merged
merged 1 commit into from
Jun 22, 2022

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Jun 22, 2022

Currently, connect_nodes will return silently when the connection is disconnected while connecting. This is confusing, so fix it.

Can be tested by reverting the signet test change and observing the failure when running the test.

Also replace the use of wait_until_helper, which is not allowed to be
called directly. Otherwise, --timeout-factor will not be honoured.
@laanwj
Copy link
Member

laanwj commented Jun 22, 2022

Tested ACK faee330
Silent failures are really bad in general, even more so in tests which are supposed to raise a stink on problems.
Tested that removing the setup_network function makes the test fail.

@laanwj laanwj merged commit 0808c88 into bitcoin:master Jun 22, 2022
@maflcko maflcko deleted the 2206-test-fail-connect-👇 branch June 22, 2022 12:31
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 22, 2022
faee330 test: Fail if connect_nodes fails (MacroFake)

Pull request description:

  Currently, `connect_nodes` will return silently when the connection is disconnected while connecting. This is confusing, so fix it.

  Can be tested by reverting the signet test change and observing the failure when running the test.

ACKs for top commit:
  laanwj:
    Tested ACK faee330

Tree-SHA512: 641ca8adcb9f5ff33239b143573bddc0dfde41dbd103751ee870f1572ca2469f6a0d4bab6693102454cd3e270ef8251d87fbfac48f6d8adac70d2d6bbffaae56
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 22, 2022
@bitcoin bitcoin locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants