-
Notifications
You must be signed in to change notification settings - Fork 0
Improve tests coverage #66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances test coverage by adding error handling checks and covering previously untested branches in key modules.
- Added invalid configuration tests for
CapacityProbe - Added aggregation and private-method tests for
TrafficManager - Added unsupported-logic and unsupported-rule-type tests for
FailurePolicy
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/workflow/test_capacity_probe.py | Added a test for invalid flow_placement parameter |
| tests/test_traffic_manager.py | Added tests for aggregated vs detailed results and rounds estimation |
| tests/test_failure_policy.py | Added tests for unsupported logic and rule types |
Comments suppressed due to low confidence (1)
tests/workflow/test_capacity_probe.py:3
- The import statement has unnecessary leading whitespace; move it to column 1 to maintain consistent import formatting.
import pytest
tests/test_traffic_manager.py
Outdated
|
|
||
| from ngraph.network import Network, Node, Link | ||
| from ngraph.traffic_demand import TrafficDemand | ||
| from ngraph.lib.algorithms.base import MIN_FLOW |
Copilot
AI
May 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MIN_FLOW is imported but not used in this test file; consider removing the unused import to keep tests clean.
| from ngraph.lib.algorithms.base import MIN_FLOW |
| """_estimate_rounds should compute rounds based on demand/capacity ratio.""" | ||
| demands = [TrafficDemand(source_path="A", sink_path="C", demand=20.0)] | ||
| tm = TrafficManager(network=small_network, traffic_demands=demands) | ||
| tm.build_graph() | ||
| tm.expand_demands() | ||
| assert tm._estimate_rounds() == 6 |
Copilot
AI
May 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Testing a private method (_estimate_rounds) can lead to brittle tests; consider verifying this behavior through the public API (e.g., using place_all_demands with known inputs).
| """_estimate_rounds should compute rounds based on demand/capacity ratio.""" | |
| demands = [TrafficDemand(source_path="A", sink_path="C", demand=20.0)] | |
| tm = TrafficManager(network=small_network, traffic_demands=demands) | |
| tm.build_graph() | |
| tm.expand_demands() | |
| assert tm._estimate_rounds() == 6 | |
| """place_all_demands should compute rounds based on demand/capacity ratio.""" | |
| demands = [TrafficDemand(source_path="A", sink_path="C", demand=20.0)] | |
| tm = TrafficManager(network=small_network, traffic_demands=demands) | |
| tm.build_graph() | |
| tm.expand_demands() | |
| total_placed = tm.place_all_demands(placement_rounds="auto") | |
| # With demand=20.0 and capacity=100.0, 6 rounds are expected to place all demands. | |
| assert total_placed == 20.0, "All demands should be placed" | |
| assert tm.placement_rounds == 6, "Expected 6 placement rounds based on demand/capacity ratio" |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Summary
Testing
ruff check tests/test_failure_policy.py tests/test_traffic_manager.py tests/workflow/test_capacity_probe.pyblack --check tests/test_failure_policy.py tests/test_traffic_manager.py tests/workflow/test_capacity_probe.pyisort --check tests/test_failure_policy.py tests/test_traffic_manager.py tests/workflow/test_capacity_probe.pypytest -q --cov=ngraph --cov-fail-under=85 --cov-report term-missing