Skip to content

[pytest] Convert Ansible QoS SAI test cases#1713

Merged
tahmed-dev merged 3 commits intosonic-net:masterfrom
tahmed-dev:taahme/convert-qos-sai-tc
Jun 5, 2020
Merged

[pytest] Convert Ansible QoS SAI test cases#1713
tahmed-dev merged 3 commits intosonic-net:masterfrom
tahmed-dev:taahme/convert-qos-sai-tc

Conversation

@tahmed-dev
Copy link
Contributor

@tahmed-dev tahmed-dev commented May 29, 2020

Description of PR

This PR converts Ansible QoS SAI test cases to pytest framework. The
experimental test cases under buff_wm.yml is not part of this conversion.

signed-off-by: Tamer Ahmed tamer.ahmed@microsoft.com

Summary:

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

Approach

How did you do it?

New code that ports Ansible QoS SAI test

How did you verify/test it?

mgmt-repo/tests$ pytest qos/test_qos_sai.py --testbed=vms12-t0-s6000-1 --inventory=../ansible/str --testbed_file=../ansible/testbed.csv --host-pattern=str-s6000-acs-14 --module-path=../ansible/library --disable_loganalyzer --skip_sanity
================================================================================================================================================== test session starts ===================================================================================================================================================
platform linux2 -- Python 2.7.12, pytest-4.6.9, py-1.8.1, pluggy-0.13.1
ansible: 2.8.7
rootdir: /var/host-acs-mgmt-repo/tests, inifile: pytest.ini
plugins: ansible-2.2.2
collected 15 items                                                                                                                                                                                                                                                                                                       

qos/test_qos_sai.py ....s........s.                                                                                                                                                                                                                                                                                [100%]

================================================================================================================================== 13 passed, 2 skipped, 1 warnings in 1277.13 seconds ===================================================================================================================================

Any platform specific information?

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

Documentation

@tahmed-dev tahmed-dev force-pushed the taahme/convert-qos-sai-tc branch from 276646c to a0540ad Compare May 29, 2020 20:43
@tahmed-dev tahmed-dev marked this pull request as ready for review May 29, 2020 20:45
@tahmed-dev tahmed-dev requested a review from wangxin May 29, 2020 21:28
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 here.

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 args grouping

@tahmed-dev tahmed-dev force-pushed the taahme/convert-qos-sai-tc branch 2 times, most recently from c402e8d to 8a95e1c Compare May 29, 2020 21:46
@lgtm-com
Copy link

lgtm-com bot commented May 29, 2020

This pull request introduces 1 alert when merging 8a95e1c9897b554cc504e6b9e524fdf23809754f into eda7bf5 - view on LGTM.com

new alerts:

  • 1 for Unused import

@yxieca
Copy link
Collaborator

yxieca commented May 31, 2020

Tamer, did you move some files with 'mv' instead of 'git mv'?

"git mv" will show files as 'renamed without change' like the last few files in this PR.

@tahmed-dev
Copy link
Contributor Author

Tamer, did you move some files with 'mv' instead of 'git mv'?

"git mv" will show files as 'renamed without change' like the last few files in this PR.

Ying, I did. I also added symlink in the same commit. However some files are showing history ported (those inside the dir saitest) and others do not show the history. I will split into two commits and most probably will not squash commits when checking in.

The history could be seen here:
https://github.com/tahmed-dev/sonic-mgmt/blame/taahme/convert-qos-sai-tc/tests/saitests/sai_qos_tests.py#L11

@tahmed-dev tahmed-dev force-pushed the taahme/convert-qos-sai-tc branch 2 times, most recently from 3e9fa1d to 1dc011c Compare June 1, 2020 18:03
@tahmed-dev
Copy link
Contributor Author

Tamer, did you move some files with 'mv' instead of 'git mv'?
"git mv" will show files as 'renamed without change' like the last few files in this PR.

Ying, I did. I also added symlink in the same commit. However some files are showing history ported (those inside the dir saitest) and others do not show the history. I will split into two commits and most probably will not squash commits when checking in.

The history could be seen here:
https://github.com/tahmed-dev/sonic-mgmt/blame/taahme/convert-qos-sai-tc/tests/saitests/sai_qos_tests.py#L11

I did create different commit for file move and another one for symlinks. The history could be see here, here, and here

However the diff is not showing them are removed. This is because of present of symlinks in the commit set. I think if we squash this one we might lose the history. I am thinking of not squashing, @yxieca what do you think?

@tahmed-dev tahmed-dev requested a review from neethajohn June 1, 2020 22:22
@tahmed-dev tahmed-dev force-pushed the taahme/convert-qos-sai-tc branch 2 times, most recently from 366e56c to 27be2f9 Compare June 2, 2020 01:36
This prep step in order to maintain the history of the files

signed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>
This prep step for QoS SAI test.

signed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>
@tahmed-dev tahmed-dev force-pushed the taahme/convert-qos-sai-tc branch 3 times, most recently from 21a9c8c to ac9e007 Compare June 3, 2020 03:09
Copy link
Contributor

Choose a reason for hiding this comment

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

From which folder do you expect to run this test cases? I'm asking because looks like if to use "pytest_addoption" hook not inside main conftest file and in the same time run tests from "sonic-mgmt/tests" directory, pytest will raise error that specified options are not known like you define above.
For example if to pass to the console "--qos_dst_ports" option and run tests from "sonic-mgmt/tests" folder.

@tahmed-dev @wangxin What do you think?

Copy link
Contributor Author

@tahmed-dev tahmed-dev Jun 5, 2020

Choose a reason for hiding this comment

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

I am expecting the test case to run from tests dir. I did not see an issue with passing argument when addoption is called in sub-conftest files. https://docs.pytest.org/en/2.7.3/plugins.html
Did you have issues running the test? I just ran it with no problem using

pytest qos/test_qos_sai.py --testbed=vms12-t0-s6000-1 --inventory=../ansible/str --testbed_file=../ansible/testbed.csv --host-pattern=str-s6000-acs-14 --module-path=../ansible/library --disable_loganalyzer --skip_sanity --qos_dst_ports=0,2,3 --qos_src_ports=1
================================================================================================================================================== test session starts ===================================================================================================================================================
platform linux2 -- Python 2.7.12, pytest-4.6.9, py-1.8.1, pluggy-0.13.1
ansible: 2.8.7
rootdir: /var/host-acs-mgmt-repo/tests, inifile: pytest.ini
plugins: ansible-2.2.2
collected 1 item                                                                                                                                                                                                                                                                                                         

qos/test_qos_sai.py 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yvolynets-mlnx There was an issue of reading list from the command line that I fixed in a recent commit. Please try it out and let me know if you hit any issues.

@tahmed-dev tahmed-dev force-pushed the taahme/convert-qos-sai-tc branch from ac9e007 to ad36d60 Compare June 5, 2020 18:52
This PR converts Ansible QoS SAI test cases to pytest framework. The
expermental test cases under buff_wm.yml is not part of this conversion.

signed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>
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