qa: Enable feature_init.py on Windows #32021

pull hodlinator wants to merge 1 commits into bitcoin:master from hodlinator:2025/03/windows_feature_init changing 1 files +18 −12

hodlinator commented at 9:33 PM on March 8, 2025: contributor

Windows has been skipped since feature_init.py was added in #23289. Possibly due to poorer support on older Python versions, or attempts to use CTRL_C_EVENT (which didn't work in my testing either) instead of CTRL_BREAK_EVENT.
qa: Enable feature_init.py on Windows 59c4930394
DrahtBot commented at 9:33 PM on March 8, 2025: contributor



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



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32021.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK maflcko, BrandonOdiwuor, hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label Tests on Mar 8, 2025
hebasto added the label Windows on Mar 9, 2025
maflcko commented at 1:59 PM on March 9, 2025: member

lgtm ACK 59c4930394cafc939eb396224b3d60d01ba0ce37
BrandonOdiwuor commented at 1:34 PM on March 11, 2025: contributor

Code Review ACK 59c4930394cafc939eb396224b3d60d01ba0ce37

Type	Reviewers
ACK	maflcko, BrandonOdiwuor, hebasto

in test/functional/feature_init.py:47 in 59c4930394

  52 | -            node.process.terminate()
  53 | +            if platform.system() == 'Windows':
  54 | +                # Don't call Python's terminate() since it calls
  55 | +                # TerminateProcess(), which unlike SIGTERM doesn't allow
  56 | +                # bitcoind to perform any shutdown logic.
  57 | +                os.kill(node.process.pid, signal.CTRL_BREAK_EVENT)

hebasto commented at 10:13 PM on March 11, 2025:

Can this approach be used for all platforms?

fanquake commented at 3:45 AM on March 12, 2025:

No. CTRL_BREAK_EVENT only exists on Windows: https://docs.python.org/3/library/signal.html#signal.CTRL_BREAK_EVENT.

hebasto approved
hebasto commented at 11:25 AM on March 12, 2025: member

ACK 59c4930394cafc939eb396224b3d60d01ba0ce37, I have reviewed the code and it looks OK.
hebasto merged this on Mar 12, 2025
hebasto closed this on Mar 12, 2025
TheCharlatan referenced this in commit 2cab7a09ea on Mar 16, 2025
stickies-v referenced this in commit d760fd3dda on Mar 17, 2025
stickies-v referenced this in commit cc83553352 on Mar 17, 2025
stickies-v referenced this in commit 2614933f06 on Mar 17, 2025
stickies-v referenced this in commit b70418c5fc on Mar 17, 2025
stickies-v referenced this in commit 69f8a1fe50 on Mar 17, 2025
hodlinator deleted the branch on Mar 24, 2025

Contributors

Labels

Linked (view graph)

#23289 test: add stress tests for initialization

qa: Enable feature_init.py on Windows #32021

Code Coverage & Benchmarks

Reviews