Skip to content

Use ptf_runner in qos sai test and fdb mac expire test#6869

Merged
liat-grozovik merged 1 commit intosonic-net:masterfrom
congh-nvidia:use_ptf_runner
Nov 28, 2022
Merged

Use ptf_runner in qos sai test and fdb mac expire test#6869
liat-grozovik merged 1 commit intosonic-net:masterfrom
congh-nvidia:use_ptf_runner

Conversation

@congh-nvidia
Copy link
Contributor

Description of PR

Summary:
Update the qos sai test and fdb mac expire test to use ptf_runner to run the ptf test, so that the ptf logs can be collected to sonic-mgmt docker and attached to allure report by the ptf_runner.
Also update the changed files to pass the pre-commit check.

Type of change

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

Back port request

  • 201911
  • 202012
  • 202205

Approach

What is the motivation for this PR?

This PR is to improve the debuggability.
Use ptf_runner to run the ptf test, so that the ptf logs can be collected to sonic-mgmt docker and attached to allure report by the ptf_runner.

How did you do it?

Replaced the ptfhost.shell() with ptf runner() in test_fdb_mac_expire.py and qos_sai_base.py, added a "custom_option" parameter in ptf_runner to accept the extra options like " --disable-ipv6" in qos sai test.
Made some minor changes to pass the pre-commit check.

How did you verify/test it?

Run regression on all Nvidia platforms, all test passed.

Any platform specific information?

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

Documentation

@lgtm-com
Copy link

lgtm-com bot commented Nov 22, 2022

This pull request introduces 1 alert and fixes 2 when merging eef4c42 into 3cf50ec - view on LGTM.com

new alerts:

  • 1 for Implicit string concatenation in a list

fixed alerts:

  • 2 for Except block handles 'BaseException'

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

@liat-grozovik liat-grozovik merged commit 9d02d15 into sonic-net:master Nov 28, 2022
from tests.common.dualtor.dual_tor_utils import upper_tor_host,lower_tor_host,dualtor_ports
from tests.common.dualtor.mux_simulator_control import toggle_all_simulator_ports, get_mux_status, check_mux_status, validate_check_result
from tests.common.dualtor.constants import UPPER_TOR, LOWER_TOR
#from tests.common.dualtor.dual_tor_utils import upper_tor_host, lower_tor_host, dualtor_ports # noqa F401
Copy link
Collaborator

Choose a reason for hiding this comment

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

@congh-nvidia Can you elaborate why comment out this import line even when the fixtures imported here are used by this script?

Copy link
Contributor Author

@congh-nvidia congh-nvidia Apr 3, 2023

Choose a reason for hiding this comment

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

Hi @wangxin, I think it must be a typo when I was fixing the pre-commit issues. This line should not be comment out. I think it didn't break the test because the fixtures are also imported in the pytest plugin. Did you find it when converting the test to Python3, if so please remove the comment; if not, I'll fix this. Thanks!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @congh-nvidia, I noticed this when I was doing cherry-pick. Not in python3 migration. It would be great if you can submit a PR to fix this. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I'll fix it, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wangxin The PR is opened: #7965. Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you @congh-nvidia !

@congh-nvidia congh-nvidia deleted the use_ptf_runner branch October 17, 2024 02:11
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.

4 participants