From a90b8664bf39770b71b1533b474c28b585b455ed Mon Sep 17 00:00:00 2001 From: navyaa14 Date: Tue, 16 Jun 2026 16:21:53 +0530 Subject: [PATCH 1/3] Fix policy type retrieval in snapmirror_operations.py --- .../traditional/ontap/snapmirror_operations.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/netapp_dataops_traditional/netapp_dataops/traditional/ontap/snapmirror_operations.py b/netapp_dataops_traditional/netapp_dataops/traditional/ontap/snapmirror_operations.py index 03dfa87..a56fd7f 100644 --- a/netapp_dataops_traditional/netapp_dataops/traditional/ontap/snapmirror_operations.py +++ b/netapp_dataops_traditional/netapp_dataops/traditional/ontap/snapmirror_operations.py @@ -81,10 +81,14 @@ def list_snap_mirror_relationships(print_output: bool = False, cluster_name: Opt healthy = relationship.healthy else: healthy = "unknown" - + try: + policy_type = relationship.policy.type + except AttributeError: + policy_type = None + relationshipDict = { "UUID": relationship.uuid, - "Type": relationship.policy.type, + "Type": policy.type, "Healthy": healthy, "Current Transfer Status": transferState, "Source Cluster": sourceCluster, From 34ddd4220bdab83317eeb84932744c76efe6a3a6 Mon Sep 17 00:00:00 2001 From: navyaa14 Date: Tue, 16 Jun 2026 16:24:47 +0530 Subject: [PATCH 2/3] Update snapmirror_operations.py --- .../netapp_dataops/traditional/ontap/snapmirror_operations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netapp_dataops_traditional/netapp_dataops/traditional/ontap/snapmirror_operations.py b/netapp_dataops_traditional/netapp_dataops/traditional/ontap/snapmirror_operations.py index a56fd7f..7610e79 100644 --- a/netapp_dataops_traditional/netapp_dataops/traditional/ontap/snapmirror_operations.py +++ b/netapp_dataops_traditional/netapp_dataops/traditional/ontap/snapmirror_operations.py @@ -88,7 +88,7 @@ def list_snap_mirror_relationships(print_output: bool = False, cluster_name: Opt relationshipDict = { "UUID": relationship.uuid, - "Type": policy.type, + "Type": policy_type, "Healthy": healthy, "Current Transfer Status": transferState, "Source Cluster": sourceCluster, From 06e5f772d9ffbca86f4264073bd0ffa6e0477897 Mon Sep 17 00:00:00 2001 From: navyaa14 Date: Tue, 16 Jun 2026 16:51:07 +0530 Subject: [PATCH 3/3] Create test_snapmirror_issue33.py --- .../tests/ontap/test_snapmirror_issue33.py | 163 ++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100644 netapp_dataops_traditional/tests/ontap/test_snapmirror_issue33.py diff --git a/netapp_dataops_traditional/tests/ontap/test_snapmirror_issue33.py b/netapp_dataops_traditional/tests/ontap/test_snapmirror_issue33.py new file mode 100644 index 0000000..77c61ad --- /dev/null +++ b/netapp_dataops_traditional/tests/ontap/test_snapmirror_issue33.py @@ -0,0 +1,163 @@ +""" +Regression tests for GitHub issue #33: + "List SnapMirror Relationships tool fails with 'policy field has not been set' error." + +Production file under test (never modified here): + netapp_dataops_traditional/netapp_dataops/traditional/ontap/snapmirror_operations.py + +These are UNIT TESTS only. All network-dependent behaviour is replaced with +mocks, so no ONTAP credentials, network access, or real NetApp cluster are +needed or contacted. + +Real SnapmirrorRelationship objects are constructed via + SnapmirrorRelationship.from_dict(...) +so that the SDK's own attribute-guard logic fires exactly as in production. +Only three I/O entry points are patched: + + _retrieve_config → returns a static config dict + _instantiate_connection → no-op, no connection attempted + NetAppSnapmirrorRelationship.get_collection + → yields one fake relationship object + +Two test functions: + + test_missing_policy_returns_none + Regression test for issue #33. + The relationship omits the 'policy' field. After the fix the function + must NOT raise; it must return a list with one entry whose "Type" is None. + + test_present_policy_returns_type + Control / happy-path test. + The relationship has policy.type == "async". The function must return + a list with one entry whose "Type" == "async". +""" + +import pytest +from unittest.mock import patch, MagicMock + +from netapp_ontap.resources import SnapmirrorRelationship as NetAppSnapmirrorRelationship +from netapp_dataops.traditional.ontap.snapmirror_operations import ( + list_snap_mirror_relationships, +) + + +# --------------------------------------------------------------------------- +# Patch-target constants — match what the production module imports +# --------------------------------------------------------------------------- +_MOD = "netapp_dataops.traditional.ontap.snapmirror_operations" +_RETRIEVE_CONFIG = f"{_MOD}._retrieve_config" +_INSTANTIATE_CONN = f"{_MOD}._instantiate_connection" +_GET_COLLECTION = f"{_MOD}.NetAppSnapmirrorRelationship.get_collection" + + +# --------------------------------------------------------------------------- +# Minimal valid config (no real credentials required) +# --------------------------------------------------------------------------- +_FAKE_CONFIG = { + "connectionType": "ONTAP", + "hostname": "fake-cluster.example.com", + "username": "admin", + "password": "fake-password", + "svm": "fake_svm", + "dataLIF": "192.0.2.1", + "verifySSLCert": False, +} + +# Minimal fields shared by both test objects +_BASE_FIELDS = { + "uuid": "aaaabbbb-1234-5678-abcd-ef0123456789", + "source": {"svm": {"name": "source_svm"}, "path": "source_svm:source_vol"}, + "destination": {"svm": {"name": "dest_svm"}, "path": "dest_svm:dest_vol"}, + "healthy": True, +} + + +# --------------------------------------------------------------------------- +# Helper: patch I/O and run the real production function +# --------------------------------------------------------------------------- + +def _run(fake_rel): + """ + Patch the three I/O entry points, inject *fake_rel* as the sole + relationship returned by get_collection(), and call the real + list_snap_mirror_relationships(). Returns the function's return value + or re-raises whatever it raises. + """ + # Patch get() on the real SDK object so it does not attempt HTTP + fake_rel.get = MagicMock(return_value=None) + + with patch(_RETRIEVE_CONFIG, return_value=_FAKE_CONFIG), \ + patch(_INSTANTIATE_CONN, return_value=None), \ + patch(_GET_COLLECTION, return_value=iter([fake_rel])): + return list_snap_mirror_relationships(print_output=False) + + +# =========================================================================== +# Test 1 — Regression: missing policy must not crash; Type must be None +# =========================================================================== + +def test_missing_policy_returns_none(): + """ + Regression test for issue #33. + + A real SnapmirrorRelationship built without a 'policy' key causes the + NetApp SDK to raise: + AttributeError: The 'policy' field has not been set ... + + After the production fix (try/except AttributeError → policy_type = None) + the function must complete successfully and return a list whose sole + entry has "Type" set to None. + + This is a mock-based unit test. No ONTAP cluster, credentials, or + network access are used. + """ + fake_rel = NetAppSnapmirrorRelationship.from_dict(_BASE_FIELDS) + + # Confirm the SDK raises as expected — documents the bug precondition + with pytest.raises(AttributeError, match="policy.*has not been set"): + _ = fake_rel.policy + + result = _run(fake_rel) + + assert len(result) == 1, f"Expected 1 relationship, got {len(result)}" + assert result[0]["Type"] is None, f"Expected Type=None, got {result[0]['Type']!r}" + + # Spot-check the other fields are still populated correctly + assert result[0]["UUID"] == "aaaabbbb-1234-5678-abcd-ef0123456789" + assert result[0]["Source SVM"] == "source_svm" + assert result[0]["Source Volume"] == "source_vol" + assert result[0]["Dest SVM"] == "dest_svm" + assert result[0]["Dest Volume"] == "dest_vol" + assert result[0]["Healthy"] is True + + +# =========================================================================== +# Test 2 — Control: present policy must pass through unchanged +# =========================================================================== + +def test_present_policy_returns_type(): + """ + Control / happy-path test. + + When policy.type IS present the function must return it unchanged. + This verifies the fix does not regress the normal code path. + + This is a mock-based unit test. No ONTAP cluster, credentials, or + network access are used. + """ + fake_rel = NetAppSnapmirrorRelationship.from_dict({ + **_BASE_FIELDS, + "policy": {"type": "async"}, + }) + + result = _run(fake_rel) + + assert len(result) == 1, f"Expected 1 relationship, got {len(result)}" + assert result[0]["Type"] == "async", f"Expected Type='async', got {result[0]['Type']!r}" + + assert result[0]["UUID"] == "aaaabbbb-1234-5678-abcd-ef0123456789" + assert result[0]["Source SVM"] == "source_svm" + assert result[0]["Source Volume"] == "source_vol" + assert result[0]["Dest SVM"] == "dest_svm" + assert result[0]["Dest Volume"] == "dest_vol" + assert result[0]["Healthy"] is True