Skip to content

[Qos]Fixture update for duthost selection in test_pfc_counter.py#16654

Merged
rlhui merged 3 commits intosonic-net:masterfrom
ansrajpu-git:test_pfc_count
Feb 27, 2025
Merged

[Qos]Fixture update for duthost selection in test_pfc_counter.py#16654
rlhui merged 3 commits intosonic-net:masterfrom
ansrajpu-git:test_pfc_count

Conversation

@ansrajpu-git
Copy link
Contributor

@ansrajpu-git ansrajpu-git commented Jan 23, 2025

Description of PR

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
    • Skipped for non-supported platforms
  • Test case improvement

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405
  • 202411

Approach

What is the motivation for this PR?

Test failures in test_pfc_counters.py

How did you do it?

Change the 'duthosts[rand_one_tgen_dut_hostname]' to 'duthosts[enum_rand_one_per_hwsku_frontend_hostname]'
and.
'fanout_graph_facts' to 'enum_fanout_graph_facts'

How did you verify/test it?

Executed t2 test for broadcom-dnx chassis & verify the results.

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ansrajpu-git
Copy link
Contributor Author

@vmittal-msft ,please review

@vmittal-msft
Copy link
Contributor

@StormLiangMS @wangxin please review for T0/T1 side.

@arista-nwolfe
Copy link
Contributor

@ansrajpu-git is it possible to add some details about what failure you were seeing that prompted this change?
We're curious if we're seeing the same issue on Arista or not.

@kenneth-arista
Copy link
Contributor

Can you provide details on failures seen running test_pfc_counters.py? Why do we need these changes?

@rlhui
Copy link

rlhui commented Feb 6, 2025

@ansrajpu-git - please add why is this change needed in description of the PR? Thanks.

@XuChen-MSFT
Copy link
Contributor

@ansrajpu-git need more background info to know if this change will impact single box device? can you help to share detail

@ansrajpu-git
Copy link
Contributor Author

@ansrajpu-git need more background info to know if this change will impact single box device? can you help to share detail

@XuChen-MSFT , the enum_rand_one_per_hwsku_frontend_hostname will select the DUT per hwsku of the frontend node. It should work fine with the selection of dut with single box as well.

Copy link
Contributor

@XuChen-MSFT XuChen-MSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checked enum_rand_one_per_hwsku_hostname logic in pytest_generate_tests, should be compatible with single box device.
and manually run qos/test_pfc_counters.py on t1 single box device, test passed.
approved.

@rlhui rlhui merged commit cec6fcb into sonic-net:master Feb 27, 2025
15 of 16 checks passed
@rlhui
Copy link

rlhui commented Feb 27, 2025

@ansrajpu-git please create a msft 202405 PR for this? thanks.

nissampa pushed a commit to nissampa/sonic-mgmt_dpu_test that referenced this pull request Mar 4, 2025
…ic-net#16654)

Test failures in test_pfc_counters.py

How did you do it?
Change the 'duthosts[rand_one_tgen_dut_hostname]' to 'duthosts[enum_rand_one_per_hwsku_frontend_hostname]'
and. 'fanout_graph_facts' to 'enum_fanout_graph_facts'
@ansrajpu-git
Copy link
Contributor Author

@ansrajpu-git please create a msft 202405 PR for this? thanks.
PR - Azure/sonic-mgmt.msft#139 created for msft-202405 branch

nnelluri-cisco pushed a commit to nnelluri-cisco/sonic-mgmt that referenced this pull request Mar 15, 2025
…ic-net#16654)

Test failures in test_pfc_counters.py

How did you do it?
Change the 'duthosts[rand_one_tgen_dut_hostname]' to 'duthosts[enum_rand_one_per_hwsku_frontend_hostname]'
and. 'fanout_graph_facts' to 'enum_fanout_graph_facts'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

7 participants