Skip to content

qa: Improve functional test support on illumos and *BSD#35216

Open
hebasto wants to merge 7 commits into
bitcoin:masterfrom
hebasto:260505-illumos-bind
Open

qa: Improve functional test support on illumos and *BSD#35216
hebasto wants to merge 7 commits into
bitcoin:masterfrom
hebasto:260505-illumos-bind

Conversation

@hebasto

@hebasto hebasto commented May 5, 2026

Copy link
Copy Markdown
Member

This PR is a follow-up to #34256. It extends functional test support to illumos-based OSes and fixes several related issues on the *BSDs.

Changes:

  • Make lsof an optional functional test dependency via a new skip_if_no_lsof helper, consistent with other optional test deps.
  • Strip the CIDR prefix length from NetBSD ifconfig output (no-op on other platforms).
  • Suppress spurious lsof warnings on NetBSD.
  • Drop OpenBSD from the platforms supported by get_bind_addrs.
  • Document the lsof Test Suite Dependency for FreeBSD and NetBSD.
  • Add support for get_bind_addrs and feature_bind_extra on illumos.

CI runs: hebasto/bitcoin-core-nightly#280.

Addresses #34256 (comment).

@hebasto hebasto added the Tests label May 5, 2026
@DrahtBot

DrahtBot commented May 5, 2026

Copy link
Copy Markdown
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/35216.

Reviews

See the guideline for information on the review process.

Type Reviewers
Concept ACK fanquake
Approach ACK l0rinc

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #35395 (doc: Improve test suite dependencies documentation by hebasto)

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 hebasto marked this pull request as draft May 5, 2026 20:47
@l0rinc

l0rinc commented May 6, 2026

Copy link
Copy Markdown
Contributor

code review ACK 9b7788b

@l0rinc

l0rinc commented May 6, 2026

Copy link
Copy Markdown
Contributor

@fanquake

Copy link
Copy Markdown
Member

Concept ACK - seems fine given such a small diff. Why is it a draft?

Do we also need to trim the CIDR suffix, see https://github.com/hebasto/bitcoin-core-nightly/actions/runs/25427210121/job/74583785372#step:9:501?

Seems like the NetBSD tests were also broken; should include those changes here?

@hebasto

hebasto commented May 26, 2026

Copy link
Copy Markdown
Member Author

Why is it a draft?

Seems like the NetBSD tests were also broken; should include those changes here?

There are a few points to do:

  1. Check the fixed tests on NetBSD locally.
  2. Update the build documentation to mention the lsof package where needed.

@hebasto hebasto force-pushed the 260505-illumos-bind branch from 9b7788b to 943b232 Compare June 26, 2026 16:21

@l0rinc l0rinc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The OpenIndiana logs show IPv6 bind failing (so it's likely not finished yet), but the change still makes sense, thanks for continuing the work on this.

approach ACK, reviewed 943b232


def skip_if_no_lsof(self):
"""Skip the running test if the lsof utility is not available on non-Linux platforms."""
if sys.platform != "linux" and shutil.which("lsof") is None:

@l0rinc l0rinc Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

b3a71f5 qa: Add skip_if_no_lsof helper and use it where needed:

nit: the sys.platform != "linux" part indicates the name should probably be adjusted.

@hebasto hebasto changed the title qa: Support get_bind_addrs and feature_bind_extra on illumos qa: Improve functional test support on illumos and *BSD Jun 27, 2026
@hebasto hebasto force-pushed the 260505-illumos-bind branch from 943b232 to a22a6e1 Compare June 27, 2026 08:55
@hebasto hebasto marked this pull request as ready for review June 27, 2026 08:55
hebasto added 7 commits June 29, 2026 15:44
Some functional tests on non-Linux platforms rely on the `lsof` utility.
However, we treat all other functional test dependencies, such as
additional Python modules, as optional, and skip dependent tests if
those are unavailable.

This change makes `lsof` optional as well.
Modern NetBSD `ifconfig` prints interface addresses in CIDR notation
(e.g. `inet 127.0.0.1/8`), unlike the other supported platforms which
print the netmask as a separate field. The trailing prefix length breaks
functional tests that expect a plain IP address.

This change is a no-op on other platforms.
On NetBSD, `lsof` can produce the following warnings:
1. "created device cache file: ..." - usually happens in a CI
environment.
2. "compiled for NetBSD release 10.0; this is 10.1." or similar - when
the `lsof` binary package is installed on a point release.

This change suppresses both warnings printed to stderr and fixes the
affected tests.
This extends support for `get_bind_addrs` and `feature_bind_extra` to
illumos-based OSes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants