qa: Minor improvement follow-ups to 35730 #36064

pull hodlinator wants to merge 4 commits into bitcoin:master from hodlinator:2026/08/35730_follow_ups changing 2 files +21 −29

hodlinator commented at 11:33 AM on August 24, 2026: contributor
- Simplify code through assert_raises() - #35730 (review)
- Move check below related check - #35730 (review)
- Warn when skipping checks - #35730 (review)
- Minor improvements in 1 commit:
  
  Log message instead of comment - #35730 (review)
  
  Drop r from string literal prefix - #35730 (review)
refactor(qa): Simplify through using assert_raises() 7e3b60584b
refactor(qa): Move check right below related check 6d570415a0
DrahtBot added the label Tests on Aug 24, 2026
DrahtBot commented at 11:34 AM on August 24, 2026: 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/36064.



Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers

ACK pinheadmz, winterrdog

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Type	Reviewers
ACK	pinheadmz, winterrdog

in test/functional/feature_init.py:402 in 69fbb023a4 outdated

 398 | @@ -399,14 +399,14 @@ def init_fd_overflow_test(self):
 399 |              try:
 400 |                  resource.setrlimit(resource.RLIMIT_NOFILE, (soft, soft))
 401 |              except (ValueError, OSError):
 402 | -                self.log.info(f"Skipping rlimit test: cannot reduce hard limit (soft={soft}, hard={hard})")
 403 | +                self.log.warning(f"Skipping rlimit test: cannot reduce hard limit (soft={soft}, hard={hard})")

pinheadmz commented at 7:05 PM on August 24, 2026:

69fbb023a416fb5b68bc6a39a724990134b97288

The other "skipping" messages added to this module in #34937 are currently info level. I agree with your motivation from SkipTest in test_framework.py, want to switch all the logs to warning here?

hodlinator commented at 7:15 PM on August 24, 2026:

Good point, fixed remaining cases in that file in latest push.

pinheadmz approved

pinheadmz commented at 7:08 PM on August 24, 2026: member

ACK 69fbb023a416fb5b68bc6a39a724990134b97288

Built and tested on arm64 macos host and in a debian container. These changes clean up the functional tests from #35730 without affecting functionality.

<details><summary>Show Signature</summary>

pinheadmz's public key is on openpgp.org

</details>

qa: Switch to warning when skipping tests

This is convention, see of example "except SkipTest" in test_framework.py.

6248331b29

qa: Minor feature_init.py improvements

* Emit log message before performing check.
* Drop needless 'r' from string literal.

290be9eafa

hodlinator force-pushed on Aug 24, 2026

pinheadmz commented at 7:20 PM on August 24, 2026: member

ACK 290be9eafa488dd49820b3bb5698391c0abbc6a2

Added a nit commit since last review

<details><summary>Show Signature</summary>

pinheadmz's public key is on openpgp.org

</details>

winterrdog commented at 8:38 PM on August 24, 2026: contributor

tACK 290be9eafa488dd49820b3bb5698391c0abbc6a2

successfully built and ran on Debian/x86_64/clang++-18. the tests ran without a flaw and did not break any pre-existing functionality.

totally fine to me
fanquake merged this on Aug 25, 2026
fanquake closed this on Aug 25, 2026
hodlinator deleted the branch on Aug 25, 2026

Contributors

Labels

Linked (view graph)

#35730 http: limit connected HTTPRemoteClients

qa: Minor improvement follow-ups to 35730 #36064

Code Coverage & Benchmarks

Reviews