Fix ASIC check in test_pfcwd_function

[bingwang-ms]

Description of PR

Summary:
The issue was introduced by PR #15772
The ASIC check is not working as expected because of the syntax error in the code. It should be in instead of ==

sonic-mgmt/tests/pfcwd/test_pfcwd_function.py

Line 757 in 21dbd80

if dut.facts['asic_type'] == ["mellanox", "cisco-8000"]:

The issue will cause PFCWD function not stable on some platform.

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?

This PR is to fix a syntax issue in test_pfcwd_function.py.

How did you do it?

Fix the syntax.

How did you verify/test it?

The change is verified on a physical testbed.

collected 1 item                                                                                                                                                                                      

pfcwd/test_pfcwd_function.py::TestPfcwdFunc::test_pfcwd_actions[bjw2-can-4600c-2]  ^H ^HPASSED                                                                                                        [100%]

Any platform specific information?

No.

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

Not a new test.

Documentation

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[bingwang-ms]

@rbpittman Can you help review? Thanks

[mssonicbld]

Cherry-pick PR to 202405: #16539

[mssonicbld]

Cherry-pick PR to 202411: #16540

[rraghav-cisco]

@bingwang-ms : This causes the failure:
UnboundLocalError: local variable 'PFC_STORM_TIMEOUT' referenced before assignment
The var:PFC_STORM_TIMEOUT is not defined for cisco-8000.

[bingwang-ms]

@bingwang-ms : This causes the failure: UnboundLocalError: local variable 'PFC_STORM_TIMEOUT' referenced before assignment The var:PFC_STORM_TIMEOUT is not defined for cisco-8000.

@rbpittman Can you please check this? Seems the variable is not defined in some code path for Cisco platform.

[bingwang-ms]

@rraghav-cisco The root cause is below code is not reachable due to syntax issue in PR #15772 . This PR fixed the issue and the the UnboundLocalError exception happened. The fix is easy, but I would like @rbpittman to take a look first.

sonic-mgmt/tests/pfcwd/test_pfcwd_function.py

Line 760 in 976fc8c

pytest_assert(wait_until(PFC_STORM_TIMEOUT, 2, 0,

[rbpittman]

@bingwang-ms
This comes from PR #16118 which conditions the variable definition back to mellanox only.
It looks like 16118 is a reversion of #14711 for the test_pfcwd_function.py file.
I requested 14711 be merged to 202405 before my #15772 PR because it had a "silent conflict" where this var would be removed and cause this issue.
Now seems the passive reversion of 14711 is causing master issues since I had that "==" bug you've fixed here.

I think the intended order for all branches would be something like:
#14711 - removes double-conditional variable definition/use
#15772 - Uses the single conditional
#16118 - Switches back to double-conditional
#16535 - (this PR) fixes "==" issue with 15772
TBD PR - Adds cisco back to the other condition.

If the 2 conditions are needed (1 for variable definitions and another for the state checking in the loop), I'd suggest to merge the boolean condition into a variable, such as:

should_wait = dut.facts['asic_type'] in ["mellanox", "cisco-8000"]
if should_wait:
    PFC_STORM...
...
if should_wait:
    pytest_assert...

might help prevent more issues on this in the future.

[bingwang-ms]

@bingwang-ms This comes from PR #16118 which conditions the variable definition back to mellanox only. It looks like 16118 is a reversion of #14711 for the test_pfcwd_function.py file. I requested 14711 be merged to 202405 before my #15772 PR because it had a "silent conflict" where this var would be removed and cause this issue. Now seems the passive reversion of 14711 is causing master issues since I had that "==" bug you've fixed here.

I think the intended order for all branches would be something like: #14711 - removes double-conditional variable definition/use #15772 - Uses the single conditional #16118 - Switches back to double-conditional #16535 - (this PR) fixes "==" issue with 15772 TBD PR - Adds cisco back to the other condition.

If the 2 conditions are needed (1 for variable definitions and another for the state checking in the loop), I'd suggest to merge the boolean condition into a variable, such as:

should_wait = dut.facts['asic_type'] in ["mellanox", "cisco-8000"]
if should_wait:
    PFC_STORM...
...
if should_wait:
    pytest_assert...

might help prevent more issues on this in the future.

Yeah, makes sense to me. Could you please do the change?

[rbpittman]

I can, though it'll take me some time to revector and get a TB setup for validation.