Skip to content

Add unit tests for conductor redfish module#2341

Open
berendt wants to merge 1 commit into
mainfrom
implement/issue-2225-redfish-unit-tests
Open

Add unit tests for conductor redfish module#2341
berendt wants to merge 1 commit into
mainfrom
implement/issue-2225-redfish-unit-tests

Conversation

@berendt

@berendt berendt commented Jun 10, 2026

Copy link
Copy Markdown
Member

Implements #2225.

Adds tests/unit/tasks/conductor/test_redfish.py, covering every
function in osism/tasks/conductor/redfish.py. Part of Tier 3 (#2199).

Change set (single commit)

Add unit tests for conductor redfish module — a new test module that
mirrors the conventions already used in tests/unit/tasks/conductor/
(SPDX header, the shared loguru_logs fixture from tests/conftest.py,
a local _has_log helper, MagicMock/patch, section separators).

What is covered

  • _normalize_redfish_data (pure function, built from inline dicts):
    None removal, dict/listjson.dumps, bool"true"/
    "false", int/floatstr(...), plain str pass-through, empty
    dict, and a mixed input combining all of the above.
  • get_resources: parametrized delegation to _get_ethernet_interfaces,
    _get_network_adapters, _get_network_device_functions (asserting the
    other two helpers are not called), plus the unsupported-resource-type
    branch that returns [] and logs a warning.
  • _get_ethernet_interfaces: no connection, top-level exception,
    system without the ethernet_interfaces attribute, ethernet_interfaces = None, two systems × two interfaces (4 normalized entries), full
    optional-attribute exposure, mac_address=None stripping, and
    per-interface exception isolation.
  • _get_network_adapters: no connection, top-level exception, chassis
    without network_adapters, two normalized adapter entries, and
    per-adapter exception isolation.
  • _get_network_device_functions: no connection, top-level exception,
    adapter missing network_device_functions (adapter-level except, then
    processing continues with the next adapter), per-device-function
    exception isolation, MAC extraction from device_func.ethernet, missing
    ethernet (MACs stripped by normalization), and adapter_id/
    adapter_name correlation on every entry.

Mocking notes

The Redfish connection chain (get_system_collection() /
get_chassis_collection()get_members() → members) is faked with
MagicMock. del mock.attr drives the hasattr(...) is-False cases.

The per-item except branches log member.identity, so the failing
member must keep identity readable while a different attribute raises.
A MagicMock cannot raise on attribute read without leaking the
descriptor to every other mock, so a small explicit _RaisesOn shim is
used for those cases (its __getattr__ raises on one named attribute and
returns AttributeError — i.e. getattr(..., None) default — for the
rest). This is a deliberate, minor divergence from the issue's
"MagicMock is sufficient" hint; the observable behaviour the issue
specifies (item skipped, warning logged, others still returned) is
unchanged.

Verification

  • black --check — clean.
  • flake8 — clean.
  • pytest / coverage: not run locally (project convention is to let the
    python-osism-unit-tests Zuul job run the suite on the PR). Line
    coverage of the target module was reviewed by hand and every line is
    exercised; please rely on the CI job for the 100 % figure.

🤖 Generated with Claude Code

Cover every function in osism/tasks/conductor/redfish.py:

- _normalize_redfish_data: None removal, dict/list JSON encoding,
  bool lowercasing, int/float stringification, string pass-through,
  empty and mixed inputs.
- get_resources: delegation to each helper and the unsupported
  resource-type warning path.
- _get_ethernet_interfaces, _get_network_adapters and
  _get_network_device_functions: missing connection, top-level
  exception, missing/None collection attributes, normalized happy
  paths and per-item exception isolation.

The Redfish connection chain is faked with MagicMock; a small
_RaisesOn shim makes a single attribute raise on access (keeping
identity readable) to exercise the per-item except branches, which
a plain MagicMock cannot do without leaking a descriptor globally.

Assisted-by: Claude:anthropic-opus-4-8
Signed-off-by: Christian Berendt <berendt@osism.tech>
@berendt berendt force-pushed the implement/issue-2225-redfish-unit-tests branch from 1c99916 to 70c0bce Compare June 10, 2026 20:31
@berendt berendt marked this pull request as ready for review June 10, 2026 20:50
@berendt berendt requested a review from ideaship June 10, 2026 20:50

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • Consider replacing the custom _RaisesOn shim with MagicMock plus side_effect/PropertyMock on the specific attributes that should raise, which would reduce bespoke test-only helpers while keeping the same behavior.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider replacing the custom `_RaisesOn` shim with `MagicMock` plus `side_effect`/`PropertyMock` on the specific attributes that should raise, which would reduce bespoke test-only helpers while keeping the same behavior.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready

Development

Successfully merging this pull request may close these issues.

2 participants