Skip to content

Add unit tests for osism/tasks/conductor/sonic/interface detection#2234

Merged
ideaship merged 1 commit intomainfrom
gh2220
Apr 29, 2026
Merged

Add unit tests for osism/tasks/conductor/sonic/interface detection#2234
ideaship merged 1 commit intomainfrom
gh2220

Conversation

@berendt
Copy link
Copy Markdown
Member

@berendt berendt commented Apr 28, 2026

Cover detect_breakout_ports and detect_port_channels in osism/tasks/conductor/sonic/interface.py with 49 test cases: early-exit paths, NetBox-format breakout for all supported speeds, SONiC-format 400G and standard breakout, and the LAG-name regex variants (named, numeric fallback, case-insensitive, unnamed).

Closes #2220

AI-assisted: Claude Code

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

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:

  • The test module is quite large and covers two distinct behaviours (detect_breakout_ports and detect_port_channels); consider splitting it into separate files (e.g. test_breakout_detection.py and test_port_channel_detection.py) to keep each test suite focused and easier to navigate.
  • Several tests construct similar port_config dictionaries inline; you could factor the common port_config shapes into small helper functions or fixtures (similar to _port_config_for_port) to reduce duplication and make intent clearer when reading each test.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The test module is quite large and covers two distinct behaviours (`detect_breakout_ports` and `detect_port_channels`); consider splitting it into separate files (e.g. `test_breakout_detection.py` and `test_port_channel_detection.py`) to keep each test suite focused and easier to navigate.
- Several tests construct similar `port_config` dictionaries inline; you could factor the common port_config shapes into small helper functions or fixtures (similar to `_port_config_for_port`) to reduce duplication and make intent clearer when reading each test.

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.

Cover detect_breakout_ports and detect_port_channels in
osism/tasks/conductor/sonic/interface.py with 49 test cases:
early-exit paths, NetBox-format breakout for all supported speeds,
SONiC-format 400G and standard breakout, and the LAG-name regex
variants (named, numeric fallback, case-insensitive, unnamed).

Tests are split across test_breakout_detection.py and
test_port_channel_detection.py, with shared NetBox-stub helpers in
_detection_helpers.py and inline port_config dicts built via a
single _port_config_for_port helper.

Closes #2220

AI-assisted: Claude Code
Signed-off-by: Christian Berendt <berendt@osism.tech>
@ideaship ideaship merged commit 93a0c46 into main Apr 29, 2026
3 checks passed
@ideaship ideaship deleted the gh2220 branch April 29, 2026 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unit tests for osism/tasks/conductor/sonic/interface.py — detect_breakout_ports & detect_port_channels

2 participants