Fix unit test follow-ups from post-merge review

[ideaship]

Summary

Post-merge review of the unit test PRs (#2200–#2234) against their
linked issues turned up 10 findings (3 correctness bugs, 3 missing/weak
tests, 2 dead code paths, 2 fixture gaps). This PR addresses all of
them, one commit per finding.

Correctness bugs

• conductor/utils: deep_compare raised KeyError when a nested
  key present in a was absent from b — fixed with b.get(key, {}).
• sonic/connections: get_connected_interfaces had a single outer
  try/except wrapping the entire interface loop; one bad interface
  aborted all subsequent processing — restructured to per-interface
  recovery with an inner try/except covering all per-interface work.
• sonic/interface: _extract_port_number_from_alias returned None
  for Eth(Port5) because the fallback was end-anchored; added an
  explicit \(Port(\d+)\) step between the primary pattern and the
  trailing-number fallback (a digit-anywhere approach was rejected as it
  misparsed aliases like QSFP28-49).

Missing / weak tests

• sonic/cache: lock acquisition was never verified per method;
  added three spy tests (get_device_interfaces, clear,
  get_cache_stats).
• sonic/interface: six convert_netbox_interface_to_sonic fallback
  tests checked only the return value, not the logger.warning call;
  added loguru_logs assertions to all six.
• sonic/interface: _convert_using_port_config breakout path where
  the base port is found but its alias yields no number was untested.
  The production code previously shared the "Could not find base port"
  warning with the genuinely-not-found case; split into a distinct
  "Could not extract port number from alias" message and added a test
  asserting it.
• data/enums: collection-openstack-core dependency chain was
  checked only for reachability; added a link-by-link test for
  keystone → neutron → wait-for-nova → octavia.

Dead code / fixture issues

• sonic/connections: len(group) > 1 guard in
  find_interconnected_devices was unreachable (devices only enter
  the graph when they have in-role peers, so groups always have ≥ 2);
  removed the guard and corrected the misleading test comment.
• sonic/interface: interface_speed was computed in
  detect_port_channels but never passed to
  convert_netbox_interface_to_sonic; removed the dead block.
• sonic/cache: autouse reset fixture called clear_interface_cache()
  but left _thread_local.interface_cache set; fixed to also delete
  the attribute so subsequent tests start clean.

Test plan

• pipenv run pytest tests/unit/ passes
• pipenv run flake8 osism/ tests/ clean
• pipenv run mypy osism/ clean

AI-assisted: Claude Code

[sourcery-ai]

Hey - I've found 1 issue

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="tests/unit/tasks/conductor/sonic/test_interface_conversion.py" line_range="529-537" />
<code_context>
     assert _convert_using_port_config("Ethernet5", 5, True, {}) == "Eth1/2/2"


+def test_convert_using_port_config_breakout_base_port_found_but_alias_has_no_number():
+    # is_breakout=True; base port Ethernet4 IS found in port_config, but its
+    # alias "no-digits" yields no extractable number.  The code should log a
+    # warning and fall back to speed-based calculation.
+    # _convert_using_speed_calculation(5, None, True):
+    #   base_port=4, subport=2, physical_port=2 → "Eth1/2/2"
+    port_config = {"Ethernet4": {"alias": "no-digits"}}
+
+    assert _convert_using_port_config("Ethernet5", 5, True, port_config) == "Eth1/2/2"
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Also assert that the warning log is emitted in this breakout fallback case.

The test docstring says this path should log a warning, but the test only checks the return value. To fully verify the behavior and prevent regressions in logging, please capture logs (e.g., with the `loguru_logs` fixture) and assert that the expected warning is emitted, consistent with the other logging tests in this file.

Suggested implementation:

```python
def test_convert_netbox_interface_standard_form_resolves_via_alias(mocker):
    assert _convert_using_port_config("Ethernet5", 5, True, {}) == "Eth1/2/2"




```

```python
def test_convert_using_port_config_breakout_base_port_found_but_alias_has_no_number(
    loguru_logs,
):
    # is_breakout=True; base port Ethernet4 IS found in port_config, but its
    # alias "no-digits" yields no extractable number. The code should log a
    # warning and fall back to speed-based calculation.
    # _convert_using_speed_calculation(5, None, True):
    #   base_port=4, subport=2, physical_port=2 → "Eth1/2/2"
    port_config = {"Ethernet4": {"alias": "no-digits"}}

    assert _convert_using_port_config("Ethernet5", 5, True, port_config) == "Eth1/2/2"
    assert _has_log(
        loguru_logs,
        "WARNING",
        "alias has no numeric component; falling back to speed-based calculation",
    )

```

The log message substring in the `_has_log` assertion must match the actual warning emitted by `_convert_using_port_config` when the alias has no numeric component. If the implementation uses a different message, adjust the substring accordingly (keeping it specific enough to avoid false positives but resilient to minor wording changes). No other structural changes are required as long as the `loguru_logs` fixture is already defined and used similarly in other tests in this file.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

suggestion (testing): Also assert that the warning log is emitted in this breakout fallback case.

The test docstring says this path should log a warning, but the test only checks the return value. To fully verify the behavior and prevent regressions in logging, please capture logs (e.g., with the loguru_logs fixture) and assert that the expected warning is emitted, consistent with the other logging tests in this file.

Suggested implementation:

def test_convert_netbox_interface_standard_form_resolves_via_alias(mocker):
    assert _convert_using_port_config("Ethernet5", 5, True, {}) == "Eth1/2/2"

def test_convert_using_port_config_breakout_base_port_found_but_alias_has_no_number(
    loguru_logs,
):
    # is_breakout=True; base port Ethernet4 IS found in port_config, but its
    # alias "no-digits" yields no extractable number. The code should log a
    # warning and fall back to speed-based calculation.
    # _convert_using_speed_calculation(5, None, True):
    #   base_port=4, subport=2, physical_port=2 → "Eth1/2/2"
    port_config = {"Ethernet4": {"alias": "no-digits"}}

    assert _convert_using_port_config("Ethernet5", 5, True, port_config) == "Eth1/2/2"
    assert _has_log(
        loguru_logs,
        "WARNING",
        "alias has no numeric component; falling back to speed-based calculation",
    )

The log message substring in the _has_log assertion must match the actual warning emitted by _convert_using_port_config when the alias has no numeric component. If the implementation uses a different message, adjust the substring accordingly (keeping it specific enough to avoid false positives but resilient to minor wording changes). No other structural changes are required as long as the loguru_logs fixture is already defined and used similarly in other tests in this file.