Unit tests for osism/tasks/conductor/sonic/{constants,bgp}.py

[berendt]

Background

Follow-up to #2192 (foundation) and PR #2193 (pytest + Zuul infrastructure). The SONiC conductor subpackage contains a lot of logic (~5.000 LOC across several modules). The two smallest and purest modules — constants.py and bgp.py — are a natural first target: no NetBox/Redis dependencies, only pure functions and lookup tables.

Scope

Add tests/unit/tasks/conductor/sonic/test_bgp.py and tests/unit/tasks/conductor/sonic/test_constants.py covering the pure functions in osism/tasks/conductor/sonic/bgp.py and the lookup tables in osism/tasks/conductor/sonic/constants.py.

Test targets

osism/tasks/conductor/sonic/constants.py

Verify invariants of the module-level tables (these are consumed by the config generator and any regression is easy to miss):

• DEFAULT_LOCAL_AS_PREFIX == 4200
• DEFAULT_SONIC_ROLES contains expected roles (spot-check: \"spine\", \"superspine\", \"leaf\", \"accessleaf\") and is sorted (the file is ordered — assert so that accidental reordering is caught)
• PORT_TYPE_TO_SPEED_MAP:
  • Every value is a non-negative int (Mbps)
  • \"virtual\" maps to 0
  • \"10gbase-t\" maps to 10000, \"100gbase-x-qsfp28\" maps to 100000, \"400gbase-x-qsfpdd\" maps to 400000 (sample checks across families)
• HIGH_SPEED_PORTS == {100000, 200000, 400000, 800000}
• Every value in HIGH_SPEED_PORTS that corresponds to an existing key in PORT_TYPE_TO_SPEED_MAP is consistent (i.e. there exists at least one port type with speed 100000/200000/400000)
• PORT_CONFIG_PATH == \"/etc/sonic/port_config\"
• SUPPORTED_HWSKUS contains no duplicates and every entry starts with \"Accton-\" (documented format)

osism/tasks/conductor/sonic/bgp.py

calculate_local_asn_from_ipv4(ipv4_address, prefix=DEFAULT_LOCAL_AS_PREFIX) — bgp.py:10

• \"192.168.45.123\" → 4200045123 (example from the docstring)
• \"192.168.45.123/32\" (CIDR suffix) → 4200045123 (prefix stripped)
• Custom prefix: calculate_local_asn_from_ipv4(\"10.0.1.2\", prefix=4201) → 4201001002
• Zero-padding: \"10.20.5.7\" → 4200005007 (3rd/4th octet padded to 3 digits)
• Invalid formats must raise ValueError:
  • \"192.168.45\" (only three octets)
  • \"not-an-ip\" (non-numeric)
  • \"192.168.45.999\" (octet out of 0–255)
  • \"\" (empty string)
• Edge case: \"0.0.0.0\" → 4200000000
• Edge case: \"255.255.255.255\" → 4200255255

calculate_minimum_as_for_group(device_group, prefix=DEFAULT_LOCAL_AS_PREFIX) — bgp.py:65

Uses device.primary_ip4 and device.name. Build simple stub objects (dataclasses or SimpleNamespace), no NetBox mocking needed.

• Group with three devices, distinct IPs → returns the minimum calculated AS
• Group where one device has invalid primary_ip4 → that device is skipped (logger.debug called), others still evaluated
• Group where all devices have invalid IPs → returns None
• Empty group → returns None
• Group where one device has primary_ip4 == None → that device is skipped

find_interconnected_spine_groups(devices, target_roles=...) — bgp.py:48

This is a deprecated thin wrapper around connections.find_interconnected_devices. Test only the delegation:

• Mock osism.tasks.conductor.sonic.connections.find_interconnected_devices via mocker.patch and assert it is called with the correct arguments; return value is passed through unchanged.
• Default target_roles=[\"spine\", \"superspine\"] is preserved when no value is passed.

Do not write a real test of the interconnection logic here — that belongs in a future issue for connections.py.

Mocking hints

• Both modules are pure — no utils, no Redis, no NetBox. The only mock needed is for the deprecated wrapper in bgp.py (patch connections.find_interconnected_devices).
• Use types.SimpleNamespace(name=\"sw1\", primary_ip4=\"192.168.45.123/32\") as a lightweight stand-in for NetBox device objects.

Definition of Done

• tests/unit/tasks/__init__.py, tests/unit/tasks/conductor/__init__.py, tests/unit/tasks/conductor/sonic/__init__.py created
• tests/unit/tasks/conductor/sonic/test_constants.py and test_bgp.py created
• All listed cases covered; pytest --cov=osism.tasks.conductor.sonic.bgp --cov=osism.tasks.conductor.sonic.constants shows 100% for both modules
• pipenv run pytest tests/unit/tasks/conductor/sonic/ passes locally
• flake8, mypy, python-black remain green
• Zuul job python-osism-unit-tests passes

Dependencies

• Part of Meta: Unit test coverage for osism/ #2199 (meta). Blocked by Introduce foundation for unit tests (pytest + Zuul CI integration) #2192 / PR Introduce foundation for unit tests (pytest + Zuul CI) #2193