qa: Fix intermittent "Unable to connect to bitcoind" errors on Windows #28509
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
hebasto
force-pushed
the
230919-subprocess
branch
from
5c9eab5 to
d8dd6cc
Compare
|
May be easier to just bump the python version from 3.9 to 3.12 to fix the bug? |
hebasto
force-pushed
the
230919-subprocess
branch
from
d8dd6cc to
3c018fc
Compare
hebasto
force-pushed
the
230919-subprocess
branch
from
3c018fc to
89cc937
Compare
|
The CI failure is #28491 and unrelated to this PR. |
hebasto
force-pushed
the
230919-subprocess
branch
from
89cc937 to
f6c3419
Compare
From Python 3.12 Release Schedule:
The currently available Python versions in the Windows 2022 image:
|
|
Which one are we using right now? |
On Windows, it is 3.11.5. |
|
Ok, so the issue is probably not due to an too-old python version. |
|
Concept ~0. A bunch of extra code in the test-framework, to fix a not-yet-identified, Windows only issue.
Can you elaborate on how this increases robustness for non-Windows platforms, if they are already working? |
If some similar issues will happen for non-Windows platform in the future, they won't break the tests. |
You mean the issues will just be hidden / less-likely to be identified & debugged? |
This PR adds additional logging and exceptions. What do you suggest? |
I would suggest we figure out why Python doesn't work on Windows, or at least, doesn't work when run in the GitHub CI, and fix it in a targeted way (while reporting the issue upstream), with the intention to drop the workaround as soon as a newer version of Python is available, rather than inject all this new code, into the test framework, where it affects all platforms. |
I started to think that the issue is specific to GHA CI as I cannot reproduce it locally. |
|
Could it make sense to disable the functional tests on Windows for pull requests and only run them on This means that issues will be caught at a later stage only, but I'd suspect they are easy to fixup post-merge. Overall this may be less work than having someone re-run the CI on all affected pull request or having people ignore the Windows CI anyway. |
|
Yea, I think this might be the right thing to do (for now). Persistent random red CI is pointless, and confusing for contributors. It's a shame that Windows Python doesn't seem to work on GitHub, but we also aren't going to make all the changes here to work around that. |
…windows in master aba4a58 ci: Only run functional tests on windows in master (Fabian Jahr) Pull request description: This idea was discussed [here](bitcoin/bitcoin#28509 (comment)). ACKs for top commit: hebasto: ACK aba4a58 Tree-SHA512: 89fd6352b585bae3538d5350b0404c216a8225fe356d408c1ebe3394e7b9a190d65639f4eef310056e020909928d7a1f2de25585c97d2ac087d1a9f72af281eb
…in master aba4a58 ci: Only run functional tests on windows in master (Fabian Jahr) Pull request description: This idea was discussed [here](bitcoin#28509 (comment)). ACKs for top commit: hebasto: ACK aba4a58 Tree-SHA512: 89fd6352b585bae3538d5350b0404c216a8225fe356d408c1ebe3394e7b9a190d65639f4eef310056e020909928d7a1f2de25585c97d2ac087d1a9f72af281eb
4 participants
During my investigation of #28411 and other similar functional test failures on Windows in CI, I found out that
bitcoin/test/functional/test_framework/test_node.py
Line 223 in abe4fed
This PR ensures a child process's progress by checking a created PID file shortly. If the check fails, another two attempts are following.
Although this PR fixes tests on Windows, the new logic is platform-agnostic and increases test robustness.
In several dozens of runs in my personal repo GHA, the only intermittent failure still happens -- #28491.
Closes #28411.