Skip to content

[pytest/pfc_asym] Convert PFC asymmetric to pytest#1534

Merged
neethajohn merged 11 commits intosonic-net:masterfrom
yvolynets-mlnx:pr_asym_pfc
Jun 9, 2020
Merged

[pytest/pfc_asym] Convert PFC asymmetric to pytest#1534
neethajohn merged 11 commits intosonic-net:masterfrom
yvolynets-mlnx:pr_asym_pfc

Conversation

@yvolynets-mlnx
Copy link
Copy Markdown
Contributor

@yvolynets-mlnx yvolynets-mlnx commented Apr 3, 2020

Description of PR

Summary: Converted Asymmetric PFC test case from Ansible to PyTest
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • [+ ] Test case(new)

Approach

How did you do it?

Converted PFC Asymmetric test case from Ansible to PyTest.
Implementation is based on the test design - sonic-net/SONiC#563

How did you verify/test it?

Tested on the local setup.

py.test --inventory ../ansible/inventory --host-pattern {SWITCH} --module-path ../ansible/library/ --testbed {SWITCH}-t0 --testbed_file ../ansible/testbed.csv pfc_asym/test_pfc_asym.py --show-capture=no --log-cli-level info -ra --server_ports_num=20 --fanout_inventory=lab

Option "--server_ports_num" is optional, default value is 20. Used if there is a need to adjust number of used server ports.

Any platform specific information?

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

T0-*

Documentation

@yvolynets-mlnx yvolynets-mlnx changed the title Converted PFC asymmetric test case from ansible to pytest. [pytest/pfc_asym] Convert PFC asymmetric to pytest Apr 3, 2020
Signed-off-by: Yuriy Volynets <yuriyv@mellanox.com>
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Apr 7, 2020

This pull request introduces 4 alerts and fixes 1 when merging 9519c6a into f8fde6d - view on LGTM.com

new alerts:

  • 3 for Unused local variable
  • 1 for Unused import

fixed alerts:

  • 1 for Unused import

@lguohan lguohan requested a review from neethajohn April 7, 2020 16:19
Copy link
Copy Markdown
Contributor

@neethajohn neethajohn left a comment

Choose a reason for hiding this comment

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

There is a lot of code in here that can be reused by other pfc tests. Can they moved into tests/common/ in a follow up PR? Like pfc storm_template, pfc_storm_runner, deploy_pfc_gen.

Comment thread ansible/roles/test/templates/pfc_storm_stop_mlnx.j2
Comment thread tests/common/helpers/general.py
Comment thread tests/pfc_asym/helpers.py Outdated
Comment thread tests/scripts/exec_template.yml
Copy link
Copy Markdown
Contributor

@neethajohn neethajohn left a comment

Choose a reason for hiding this comment

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

Also please fix the LGTM alerts

@neethajohn
Copy link
Copy Markdown
Contributor

Can you move this common code commit (fanout, conn graph etc ) to a separate PR?

@yvolynets-mlnx
Copy link
Copy Markdown
Contributor Author

yvolynets-mlnx commented May 14, 2020

I've moved fanout code to the new PR.

Depends on PR - #1674.
Merge current PR after #1674 will be merged.

* Created separate PR which adds support of Onyx OS based fanout - sonic-net#1674
…asym_pfc

Conflicts:
	tests/common/devices.py
	tests/conftest.py
@yvolynets-mlnx
Copy link
Copy Markdown
Contributor Author

There is a lot of code in here that can be reused by other pfc tests. Can they moved into tests/common/ in a follow up PR? Like pfc storm_template, pfc_storm_runner, deploy_pfc_gen.

Moved.

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented May 14, 2020

This pull request introduces 4 alerts when merging 3549ddd into f11d076 - view on LGTM.com

new alerts:

  • 3 for Unused import
  • 1 for 'import *' may pollute namespace

Signed-off-by: Yuriy Volynets <yuriyv@mellanox.com>
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented May 14, 2020

This pull request introduces 1 alert when merging 307469c into f11d076 - view on LGTM.com

new alerts:

  • 1 for 'import *' may pollute namespace

Copy link
Copy Markdown
Contributor

@neethajohn neethajohn left a comment

Choose a reason for hiding this comment

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

fanout_graph_facts returns a dict now (#1730). Please change the usage in the script accordingly

Signed-off-by: Yuriy Volynets <yuriyv@mellanox.com>
@yvolynets-mlnx
Copy link
Copy Markdown
Contributor Author

fanout_graph_facts returns a dict now (#1730). Please change the usage in the script accordingly

Fixed.

@yvolynets-mlnx
Copy link
Copy Markdown
Contributor Author

@neethajohn Please review latest updates, and merge if finished

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Jun 5, 2020

This pull request introduces 2 alerts when merging 54504a8 into 8c8a826 - view on LGTM.com

new alerts:

  • 1 for Unused import
  • 1 for 'import *' may pollute namespace

@yvolynets-mlnx
Copy link
Copy Markdown
Contributor Author

This pull request introduces 2 alerts when merging 54504a8 into 8c8a826 - view on LGTM.com

new alerts:

  • 1 for Unused import
  • 1 for 'import *' may pollute namespace

This 2 changes done intentionally, so please skip them.

@neethajohn
Copy link
Copy Markdown
Contributor

LGTM. can you fix the merge conflicts?

Signed-off-by: Yuriy Volynets <yuriyv@mellanox.com>
…asym_pfc

Conflicts:
	tests/common/devices.py
	tests/conftest.py
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Jun 9, 2020

This pull request introduces 2 alerts when merging af4f86e into b2017a6 - view on LGTM.com

new alerts:

  • 1 for Unused import
  • 1 for 'import *' may pollute namespace

@neethajohn
Copy link
Copy Markdown
Contributor

retest this please

@yvolynets-mlnx
Copy link
Copy Markdown
Contributor Author

yvolynets-mlnx commented Jun 9, 2020

retest this please

I've tested :)

NJ> That comment was not for you. Danny is piloting sonic-mgmt PR test. :)

@neethajohn neethajohn merged commit bd8715f into sonic-net:master Jun 9, 2020
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
7f50b9815e14d90c02d9dce63fd08d90e25cee3f (HEAD -> 201911, origin/201911) handled update() function of fdb orchagent for FDB FLUSH event (sonic-net#1534)
17adc13b6ca21846fe27c94d6a16f9909c712d77 Add a check for warm-restart, and do a clear only when warm-restart is enable. (sonic-net#1498)
d097260a5aa7bd611babd5062e220056374e23d8 Fixed compilation failure with debug option (sonic-net#1518)

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants