Skip to content

Comments

Fixing test errors in test_ecn_config_update.py#1024

Merged
r12f merged 1 commit intoAzure:202412from
mramezani95:mramezani/test_ecn_config_update_wred_green_enable
Feb 24, 2026
Merged

Fixing test errors in test_ecn_config_update.py#1024
r12f merged 1 commit intoAzure:202412from
mramezani95:mramezani/test_ecn_config_update_wred_green_enable

Conversation

@mramezani95
Copy link

When updating green_min_threshold, green_max_threshold, or green_drop_probability in WRED profiles for which wred_green_enable is false (or not set) using the apply-patch command, sometimes we see these errors:

ERR syncd#syncd: [none] SAI_API_PORT:_brcm_sai_cosq_gport_discard_set:6205 cosq gport discard set failed with error Invalid parameter (0xfffffffc)
ERR syncd#syncd: [none] SAI_API_QUEUE:_brcm_sai_qos_queue_wred_set:1760 sai cosq gport discard set failed with error -5.
ERR syncd#syncd: [none] SAI_API_WRED:_brcm_xgs_sai_reapply_wred_profile:882 Apply queue config failed with error -5.

This PR fixes this issue by modifying test_ecn_config_update.py to not generate JSON patches for WRED profiles for which wred_green_enable is false (or not set). After this fix, these tests passed on a device on which the same tests failed before the fix:

generic_config_updater/test_ecn_config_update.py::test_ecn_config_updates[replace-green_min_threshold] PASSED                                         [ 25%]
generic_config_updater/test_ecn_config_update.py::test_ecn_config_updates[replace-green_max_threshold] PASSED                                         [ 50%]
generic_config_updater/test_ecn_config_update.py::test_ecn_config_updates[replace-green_drop_probability] PASSED                                      [ 75%]
generic_config_updater/test_ecn_config_update.py::test_ecn_config_updates[replace-green_min_threshold,green_max_threshold,green_drop_probability] PASSED [100%]

Signed-off-by: Mahdi Ramezani <mramezani@microsoft.com>
Copy link

Copilot AI left a 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 fixes test errors in test_ecn_config_update.py that occurred when updating WRED green parameters (green_min_threshold, green_max_threshold, green_drop_probability) for profiles where wred_green_enable is false. The fix prevents JSON patches from being generated for WRED profiles that don't have green WRED enabled, avoiding SAI API errors.

Changes:

  • Modified determine_delta_values to accept wred_green_enabled parameter and return zero deltas when green WRED is disabled
  • Added filtering logic to skip WRED profiles with wred_green_enable=false when generating JSON patches
  • Introduced a pytest fixture for tmpfile management (replacing try-finally pattern)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@r12f
Copy link
Contributor

r12f commented Feb 22, 2026

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@r12f r12f merged commit 96cd595 into Azure:202412 Feb 24, 2026
20 checks passed
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.

3 participants