Add unit tests for conductor redfish module#2341
Open
berendt wants to merge 1 commit into
Open
Conversation
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>
1c99916 to
70c0bce
Compare
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider replacing the custom
_RaisesOnshim withMagicMockplusside_effect/PropertyMockon 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.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements #2225.
Adds
tests/unit/tasks/conductor/test_redfish.py, covering everyfunction 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 thatmirrors the conventions already used in
tests/unit/tasks/conductor/(SPDX header, the shared
loguru_logsfixture fromtests/conftest.py,a local
_has_loghelper,MagicMock/patch, section separators).What is covered
_normalize_redfish_data(pure function, built from inline dicts):Noneremoval,dict/list→json.dumps,bool→"true"/"false",int/float→str(...), plainstrpass-through, emptydict, 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 theother 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_interfacesattribute,ethernet_interfaces = None, two systems × two interfaces (4 normalized entries), fulloptional-attribute exposure,
mac_address=Nonestripping, andper-interface exception isolation.
_get_network_adapters: no connection, top-level exception, chassiswithout
network_adapters, two normalized adapter entries, andper-adapter exception isolation.
_get_network_device_functions: no connection, top-level exception,adapter missing
network_device_functions(adapter-levelexcept, thenprocessing continues with the next adapter), per-device-function
exception isolation, MAC extraction from
device_func.ethernet, missingethernet(MACs stripped by normalization), andadapter_id/adapter_namecorrelation on every entry.Mocking notes
The Redfish connection chain (
get_system_collection()/get_chassis_collection()→get_members()→ members) is faked withMagicMock.del mock.attrdrives thehasattr(...)is-Falsecases.The per-item
exceptbranches logmember.identity, so the failingmember must keep
identityreadable while a different attribute raises.A
MagicMockcannot raise on attribute read without leaking thedescriptor to every other mock, so a small explicit
_RaisesOnshim isused for those cases (its
__getattr__raises on one named attribute andreturns
AttributeError— i.e.getattr(..., None)default — for therest). This is a deliberate, minor divergence from the issue's
"
MagicMockis sufficient" hint; the observable behaviour the issuespecifies (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 thepython-osism-unit-testsZuul job run the suite on the PR). Linecoverage 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