Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Jun 10, 2022

Otherwise the test will fail if two tests running in parallel use the same port. See #25096 (comment) and #25312

@fanquake fanquake added the Tests label Jun 10, 2022
@maflcko
Copy link
Member Author

maflcko commented Jun 10, 2022

The failure would only be intermittent (maybe with a probability of 1%?)

@maflcko maflcko force-pushed the 2206-test-collide- branch from fa46ea0 to fa7a711 Compare June 10, 2022 13:56
Copy link
Member

@dergoegge dergoegge left a comment

Choose a reason for hiding this comment

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

ACK fa7a711 - This gets rid of some rather arbitrary choices for ports in some of our functional tests that can cause port collisions across test runs, resulting in intermittent failures.

Using left over p2p ports (p2p_port(n)) for custom ports seems OK. In the long run (if needed) we could consider adding a third port range to the tests for custom ports (as suggested in #25096 (comment)).

@maflcko maflcko merged commit b71d37d into bitcoin:master Jun 10, 2022
@maflcko maflcko deleted the 2206-test-collide-🚉 branch June 10, 2022 15:23
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 13, 2022
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Apr 27, 2023
Summary:
Otherwise the test will fail if two tests running in parallel use the same port.

Backport of [[bitcoin/bitcoin#25333 | core#25333]] and [[bitcoin/bitcoin#25312 | core#25312]].

Test Plan:
  ./test/functional/test_runner.py feature_proxy feature_bind_extra p2p_getaddr_caching

Reviewers: #bitcoin_abc, PiRK

Reviewed By: #bitcoin_abc, PiRK

Differential Revision: https://reviews.bitcoinabc.org/D13810
@bitcoin bitcoin locked and limited conversation to collaborators Jun 10, 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