Skip to content

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.
@maflcko maflcko force-pushed the 2206-test-fail-connect- branch from faac5d9 to faee330 Compare June 22, 2022 07:15
@DrahtBot DrahtBot added the Tests label Jun 22, 2022
@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.

3 participants