Skip to content

test: add unit tests for static helpers in fabrics.c#3277

Merged
igaw merged 1 commit intolinux-nvme:masterfrom
martin-belanger:fabrics_static_funcs_unit_tests
Apr 16, 2026
Merged

test: add unit tests for static helpers in fabrics.c#3277
igaw merged 1 commit intolinux-nvme:masterfrom
martin-belanger:fabrics_static_funcs_unit_tests

Conversation

@martin-belanger
Copy link
Copy Markdown

@igaw - I thought it would be a good idea to add unit tests for the static functions in fabrics.c including traddr_is_hostname().

Adds libnvme/test/test-fabrics.c, which tests the static helper functions in src/nvme/fabrics.c without modifying any production source.

The technique is to define static to nothing and then #include the source file directly. This exposes all translation-unit-local symbols in the test binary. No #ifdef UNIT_TEST guards or visibility changes are needed — the production files are left completely untouched.

Functions covered:
strchomp, hostid_from_hostnqn, __add_bool_argument, __add_hex_argument, _add_int_argument, __add_int_or_minus_one_argument, __add_argument, inet4_pton, inet_pton_with_scope, traddr_is_hostname, unescape_uri

The test is gated on want_fabrics in meson.build, matching the conditional that guards the uriparser test above it.

The traddr_is_hostname tests include the key regression case: scoped IPv6 addresses (e.g. "fe80::1%lo") must not be mis-classified as hostnames. A plain inet_pton() call would fail on the % delimiter and wrongly return true; inet_pton_with_scope() handles it correctly.

@martin-belanger martin-belanger force-pushed the fabrics_static_funcs_unit_tests branch from 98688cd to c492a6e Compare April 16, 2026 10:54
Adds libnvme/test/test-fabrics.c, which tests the static helper
functions in src/nvme/fabrics.c without modifying any production source.

The technique is to define 'static' to nothing and then #include the
source file directly.  This exposes all translation-unit-local symbols
in the test binary.  No #ifdef UNIT_TEST guards or visibility changes
are needed — the production files are left completely untouched.

Functions covered:
  strchomp, hostid_from_hostnqn, __add_bool_argument,
  __add_hex_argument, __add_int_argument,
  __add_int_or_minus_one_argument, __add_argument,
  inet4_pton, inet_pton_with_scope, traddr_is_hostname, unescape_uri

The test is gated on want_fabrics in meson.build, matching the
conditional that guards the uriparser test above it.

Signed-off-by: Martin Belanger <martin.belanger@dell.com>
@martin-belanger martin-belanger force-pushed the fabrics_static_funcs_unit_tests branch from c492a6e to 0e18179 Compare April 16, 2026 10:58
@igaw igaw merged commit 82e3e0b into linux-nvme:master Apr 16, 2026
29 checks passed
@igaw
Copy link
Copy Markdown
Collaborator

igaw commented Apr 16, 2026

Thanks. Let's see how this goes in the long run. I do think having proper unit tests also for such functions is not a bad thing. Though it has the change to get out of hand. But that's tomorrow's problem.

@martin-belanger martin-belanger deleted the fabrics_static_funcs_unit_tests branch April 17, 2026 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants