[mmu probing] pr07.test: Add comprehensive unit tests for probe framework#22545
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
yxieca
left a comment
There was a problem hiding this comment.
Deep review done. Unit tests look fine; no issues found.
yxieca
left a comment
There was a problem hiding this comment.
Re-reviewed updates; no new issues found.
|
Below is sample test log for running this unit test for probe framework: |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Added 10 unit tests to improve bug fix coverage (
UT total: 390 → 400. See PR #22540 for the corresponding source code fixes. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Added 15 unit tests for anti-oscillation and BadSpot executors ( BadSpot executor UTs (+8):
Anti-oscillation backtrack UTs (+7):
Registry fix: hardcoded executor count -> issubset check (supports new executors) UT total: 400 -> 415. Validates algorithm fix in PR #22540 ( |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…ing Exception Physical executors catch hardware errors and return (False, False). Algorithm callers expect a tuple and check 'if not success:' without try/except. Raising Exception crashes the algorithm instead of triggering graceful retry/backtrack. Changed: raise Exception(error_msg) → return (False, False) Files: sim_pfc_xoff_probing_executor.py, sim_ingress_drop_probing_executor.py UT coverage: PR sonic-net#22545 (74b3efb) — 4 new tests verifying the return contract. Addresses @StormLiangMS review: sim intermittent raises exceptions instead of returning (False, False) like physical executors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Added 4 UTs for intermittent executor return contract (
Validates fix in PR #22541 ( UT total: 431 → 435. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Refactored multi-PG probe loop from 6 scattered 'continue' statements to while-True single-pass block with unified cleanup: - break + fail_reason on any phase failure - pg_success flag tracks completion - Single drain_buffer([dst_port_id]) call in cleanup block This ensures buffer state is always drained before moving to the next PG, preventing corrupted buffer from affecting subsequent PG probing. UT coverage: PR sonic-net#22545 (3d75029) — 7 new tests IT coverage: PR sonic-net#22546 (14a29c2) — 2 new tests Addresses @StormLiangMS review: continue on PG failure skips buffer cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Added 7 UTs for buffer cleanup on PG failure ( Tests verify
Validates fix in PR #22544 ( UT total: 435 → 442. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
When candidate_threshold is small (e.g. 10), precision target candidate * 0.05 = 0.5 < 1. With bad_spot at the threshold value, range_size stays at 1 but 1 <= 0.5 is never satisfied, burning all 50 max_iterations. Use max(1, ...) to ensure precision check can terminate when range narrows to 1 packet granularity. Validated by UT (PR sonic-net#22545) and IT (PR sonic-net#22546) — both FAIL without this fix (50 iterations), PASS with fix (~18 iterations). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen <xuchen3@microsoft.com>
…p executors
PfcXoff used explicit all_true/all_false/all_equal pattern (8 lines).
IngressDrop used concise set-dedup pattern (3 lines). Both produce
identical outcomes for all input combinations.
Unified to the concise pattern with descriptive comment:
return_result = (True, results[0])
# Multiple attempts: check consistency (set dedup detects mixed True/False)
if len(results) > 1 and len(set(results)) > 1:
return_result = (False, False)
UT coverage: PR sonic-net#22545 (e2fca74) — 12 new tests covering all 6 input
combinations for both executors.
Addresses @StormLiangMS review: inconsistent result patterns.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Xu Chen <xuchen3@microsoft.com>
…ing Exception Physical executors catch hardware errors and return (False, False). Algorithm callers expect a tuple and check 'if not success:' without try/except. Raising Exception crashes the algorithm instead of triggering graceful retry/backtrack. Changed: raise Exception(error_msg) → return (False, False) Files: sim_pfc_xoff_probing_executor.py, sim_ingress_drop_probing_executor.py UT coverage: PR sonic-net#22545 (74b3efb) — 4 new tests verifying the return contract. Addresses @StormLiangMS review: sim intermittent raises exceptions instead of returning (False, False) like physical executors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen <xuchen3@microsoft.com>
Refactored multi-PG probe loop from 6 scattered 'continue' statements to while-True single-pass block with unified cleanup: - break + fail_reason on any phase failure - pg_success flag tracks completion - Single drain_buffer([dst_port_id]) call in cleanup block This ensures buffer state is always drained before moving to the next PG, preventing corrupted buffer from affecting subsequent PG probing. UT coverage: PR sonic-net#22545 (3d75029) — 7 new tests IT coverage: PR sonic-net#22546 (14a29c2) — 2 new tests Addresses @StormLiangMS review: continue on PG failure skips buffer cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen <xuchen3@microsoft.com>
Implement complete unit test suite for all probe framework components using pytest with mock-based testing approach. Test Infrastructure: - conftest.py: Shared pytest fixtures and test utilities - pytest.ini: Pytest configuration for unit test execution Component Tests (22 test modules): Data Structures & Protocols: - test_iteration_outcome.py: Iteration result enumeration tests - test_probing_result.py: Threshold result data class tests - test_probing_executor_protocol.py: Executor protocol interface tests - test_observer_config.py: Observer configuration tests Algorithms: - test_lower_bound_probing_algorithm.py: Lower bound binary search tests - test_upper_bound_probing_algorithm.py: Upper bound binary search tests - test_threshold_point_probing_algorithm.py: Precise threshold tests - test_threshold_range_probing_algorithm.py: Threshold range tests Infrastructure: - test_executor_registry.py: Executor factory pattern tests - test_stream_manager.py: Traffic stream management tests - test_buffer_occupancy_controller.py: Buffer control tests - test_probing_observer.py: Observer pattern implementation tests Executors: - test_pfc_xoff_probing_executor.py: Physical PFC executor tests - test_sim_pfc_xoff_probing_executor.py: Simulated PFC executor tests - test_ingress_drop_probing_executor.py: Physical drop executor tests - test_sim_ingress_drop_probing_executor.py: Simulated drop executor tests Framework & Implementations: - test_probing_base.py: Base framework template method tests - test_pfc_xoff_probing.py: PFC Xoff probing implementation tests - test_ingress_drop_probing.py: Ingress drop probing tests - test_headroom_pool_probing.py: Headroom pool probing tests All tests use mock executors and dependency injection for isolation, achieving comprehensive coverage of framework functionality. Signed-off-by: Xu Chen <xuchen3@microsoft.com>
Signed-off-by: Xu Chen <xuchen3@microsoft.com>
Add UT coverage for 4 fixed issues: - is_range None guard: 3 tests (None lower, None upper, both None) - is_point None guard: 3 tests (same patterns as is_range) - Lower-bound infinite loop at current=1: 1 test (verify <= 5 iterations) - Point algo buffer drain after failure: 1 test (verify drain_buffer=True) - __repr__ with invalid state: 2 tests (both None, partial None) UT total: 390 -> 400 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen <xuchen3@microsoft.com>
BadSpot executor UTs (+8): - PFC XOFF: normal values, bad values, empty, repeated (4 tests) - Ingress Drop: same pattern (4 tests) Anti-oscillation backtrack UTs (+7): - Scenario 1-2: single-layer backtrack (parent unreached/reached) - Scenario 3-4: multi-layer backtrack (grandparent unreached/reached) - Bad region convergence: algorithm navigates around 20-value bad region - Nudge size: proportional calculation verification - Oscillation: bad value not retested more than 3 times Registry fix: hardcoded executor count → issubset check UT total: 400 -> 415 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen <xuchen3@microsoft.com>
New UT cases (+2): - test_precision_check_at_small_threshold_with_bad_spot: threshold=1 with bad_spot, verifies algorithm exits via precision_reached not max_iterations. Without fix: 50 iterations FAIL. With fix: PASS. - test_precision_check_at_small_threshold: small threshold=5 convergence Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen <xuchen3@microsoft.com>
2 new tests (order 8817-8818) covering the observer None guard fix: - test_check_verbose_observer_none_guard_result_path: verifies L281 guard - test_check_verbose_observer_none_guard_exception_path: verifies L290 guard Both tests bypass the L162 assert by nullifying observer mid-execution via counter read side_effect, then verify no AttributeError crash. Related: PR sonic-net#22541 fix (observer None guard) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen <xuchen3@microsoft.com>
…stry tests The autouse fixture in conftest.py (reset_executor_registry) already calls ExecutorRegistry.clear_registry() and cleans sys.modules before and after each test via yield (guaranteed by pytest even on test failure). Removed 6 redundant manual del/discard statements that were pre-fixture legacy code. Updated docstring to document the fixture-based isolation. Addresses @StormLiangMS review: class-level mutable state cleanup concern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen <xuchen3@microsoft.com>
6 UTs for IngressDropProbingExecutor (order 8819-8824): single detected/not-detected, multi all/none/inconsistent-TF/FT 6 UTs for PfcXoffProbingExecutor (order 8719-8724): same 6 input combinations Covers all result analysis code paths for the consistency pattern unification in PR sonic-net#22541. All 6 scenarios produce identical outcomes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen <xuchen3@microsoft.com>
2 bug reproduction tests (failure_rate=1.0): - test_pfc_xoff_intermittent_returns_tuple_not_exception (order 12) - test_ingress_drop_intermittent_returns_tuple_not_exception (order 14) Verify check() returns (False, False) on failure, not raise Exception. 2 success path tests (failure_rate=0.0): - test_pfc_xoff_intermittent_success_path (order 13) - test_ingress_drop_intermittent_success_path (order 15) Verify normal detection works when no failure occurs. Related: PR sonic-net#22541 fix (intermittent raise → return) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen <xuchen3@microsoft.com>
…bing Tests verify drain_buffer() is called before continue when a PG phase fails in the multi-PG loop (order 4250-4256): - PFC upper/lower/range failure (3 tests) - Ingress Drop upper/lower/range failure (3 tests) - Different dst_ports isolation (1 test) Uses per-type mock side_effects to correctly handle 2-tuple (upper/lower) vs 3-tuple (range/point) algorithm return values. Related: PR sonic-net#22544 fix (while-True unified cleanup) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xu Chen <xuchen3@microsoft.com>
- E127: fix continuation line indentation in test_headroom_pool_probing.py - F811: remove redundant local imports of SimIngressDropProbingExecutorIntermittent and SimPfcXoffProbingExecutorIntermittent (already imported at module level) - EOF: add trailing newline to test_ingress_drop_probing_executor.py Signed-off-by: Xu Chen <xuchen3@microsoft.com>
d901a80 to
d4509f5
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
StormLiangMS
left a comment
There was a problem hiding this comment.
✅ LGTM — Comprehensive unit test suite
22 test files covering all framework components with >90% coverage target:
- Good test isolation via
conftest.pywithautouseregistry reset andsys.modulescleanup pytest_sessionstarthook for__pycache__cleanup prevents stale bytecode issuespytest-orderensures correct test execution sequence- Coverage config with branch tracking and term-missing report
No issues found.
…work (sonic-net#22545) Description of PR Summary: Implement complete unit test suite for all probe framework components using pytest with mock-based testing approach. Test Infrastructure: conftest.py: Shared pytest fixtures and test utilities pytest.ini: Pytest configuration for unit test execution Component Tests (22 test modules): Data Structures & Protocols: test_iteration_outcome.py: Iteration result enumeration tests test_probing_result.py: Threshold result data class tests test_probing_executor_protocol.py: Executor protocol interface tests test_observer_config.py: Observer configuration tests Algorithms: test_lower_bound_probing_algorithm.py: Lower bound binary search tests test_upper_bound_probing_algorithm.py: Upper bound binary search tests test_threshold_point_probing_algorithm.py: Precise threshold tests test_threshold_range_probing_algorithm.py: Threshold range tests Infrastructure: test_executor_registry.py: Executor factory pattern tests test_stream_manager.py: Traffic stream management tests test_buffer_occupancy_controller.py: Buffer control tests test_probing_observer.py: Observer pattern implementation tests Executors: test_pfc_xoff_probing_executor.py: Physical PFC executor tests test_sim_pfc_xoff_probing_executor.py: Simulated PFC executor tests test_ingress_drop_probing_executor.py: Physical drop executor tests test_sim_ingress_drop_probing_executor.py: Simulated drop executor tests Framework & Implementations: test_probing_base.py: Base framework template method tests test_pfc_xoff_probing.py: PFC Xoff probing implementation tests test_ingress_drop_probing.py: Ingress drop probing tests test_headroom_pool_probing.py: Headroom pool probing tests All tests use mock executors and dependency injection for isolation, achieving comprehensive coverage of framework functionality. Signed-off-by: Xu Chen <xuchen3@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description of PR
Summary:
Implement complete unit test suite for all probe framework components using pytest with mock-based testing approach.
Test Infrastructure:
Component Tests (22 test modules):
Data Structures & Protocols:
Algorithms:
Infrastructure:
Executors:
Framework & Implementations:
All tests use mock executors and dependency injection for isolation, achieving comprehensive coverage of framework functionality.
Fixes # (issue)
Type of change
Back port request
Approach
What is the motivation for this PR?
qos refactoring
How did you do it?
How did you verify/test it?
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation
relevant PRs:
[mmu probing] pr01.docs: Add MMU threshold probing framework design
[mmu probing] pr02.probe: Add core probing algorithms with essential data structures
[mmu probing] pr03.probe: Add probing executors and executor registry
[mmu probing] pr04.probe: Add observer pattern for metrics tracking
[mmu probing] pr05.probe: Add stream manager and buffer occupancy controller
[mmu probing] pr06.probe: Add base framework and all probing implementations
[mmu probing] pr07.test: Add comprehensive unit tests for probe framework
[mmu probing] pr08.test: Add integration tests for end-to-end probing workflows
[mmu probing] pr09.test: Add production probe test and infrastructure updates