Skip to content

Add conditional check to skip snmp lldp pytest case when topo is ptf32 or ptf64.#1468

Merged
lguohan merged 3 commits intosonic-net:masterfrom
EmmaLin11:master
Mar 26, 2020
Merged

Add conditional check to skip snmp lldp pytest case when topo is ptf32 or ptf64.#1468
lguohan merged 3 commits intosonic-net:masterfrom
EmmaLin11:master

Conversation

@EmmaLin11
Copy link
Contributor

@EmmaLin11 EmmaLin11 commented Mar 19, 2020

Description of PR

Summary:
Add conditional check to skip snmp lldp pytest case when topo is ptf32 or ptf64.

When topo is ptf32 or ptf64, snmp lldp pytest will failed. Because there are no lldp data respond from sonic device. The topologies for lldp does not include ptf32 and ptf64 on testcase.yml. Thus the ptf32 and ptf64 topo shall be skipped.

Type of change

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

Approach

How did you do it?

Add conditional check to skip snmp lldp pytest case when topo is ptf32 or ptf64

How did you verify/test it?

Perform pytest to confirm snmp lldp is skipped when topo is ptf32 or ptf64.

sonic@2501de585901:~/sonic-mgmt/tests$ py.test --inventory=lab --host-pattern=2-9_ptf32  --module-path ../ansible/library/ --testbed=2-9_ptf32  --testbed_file =testbed.csv snmp/test_snmp_lldp.py -log-cli-level=INFO --log-level=DEBUG -v  --show-capture=stdout --duration=0
==================================================================== test session starts =====================================================================
platform linux2 -- Python 2.7.12, pytest-4.6.7, py-1.8.0, pluggy-0.13.1 -- /usr/bin/python
cachedir: .pytest_cache
ansible: 2.8.7
rootdir: /var/sonic/sonic-mgmt/tests, inifile: pytest.ini
plugins: ansible-2.2.2
collecting ... collected 1 item                                                                                                                                             

snmp/test_snmp_lldp.py::test_snmp_interfaces SKIPPED                                                                                                   [100%]

=================================================================== slowest test durations ===================================================================
1.66s setup    snmp/test_snmp_lldp.py::test_snmp_interfaces

(0.00 durations hidden.  Use -vv to show these durations.)
================================================================= 1 skipped in 1.67 seconds ==================================================================

Any platform specific information?

N/A

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

N/A

Documentation

N/A

…o is ptf32 or ptf64.

When topo is ptf32 or ptf64, snmp lldp pytest will failed. Because there are no lldp data respond from sonic device. The topologies for lldp does not include ptf32 and ptf64 on testcase.yml. Thus the ptf32 and ptf64 topo shall be skipped.

Signed-off-by: Emma Lin <[email protected]>
@lguohan
Copy link
Contributor

lguohan commented Mar 20, 2020

@wangxin , I do not know if this is the best way to skip a test for a testbed. I do not like ptf32, ptf64, too many ptf, we should at least skip all ptf testbeds. maybe it is better to define a function to check the testbed type, only ptf, t0, t1. then, we can then filter the test based on testbed type.


@pytest.fixture(scope="module", autouse=True)
def setup_check_topo(testbed):
if testbed['topo']['name'] in ('ptf32','ptf64'):
Copy link
Contributor

Choose a reason for hiding this comment

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

this is difficult to maintain. we should at least define a function get_testbed_type(), and only three type should be returned t1, t0, and ptf.

Copy link
Contributor Author

@EmmaLin11 EmmaLin11 Mar 23, 2020

Choose a reason for hiding this comment

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

@lguohan , I get it. I defined get_testbed_type() for it. But I have some questions. What is the type of this function? It's a common function? If yes, where should I define this function? Could you give me some advice? Thanks.

Copy link
Collaborator

@wangxin wangxin Mar 23, 2020

Choose a reason for hiding this comment

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

Would you please consider adding a field to the object created in class TestbddInfo which is defined in tests/conftest.py. For example:

class TestbedInfo(object):
    ...

def __init__(self, testbed_file):
    ...
    line['topo']['type'] = self.get_testbed_type()

def get_testbed_type(self):
    ...

Then in test scripts, you can access it using testbed['topo']['type'].

Copy link
Contributor Author

@EmmaLin11 EmmaLin11 Mar 24, 2020

Choose a reason for hiding this comment

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

@wangxin , thank you for your advice. I added a field 'type' to the object created in class TestbddInfo which is defined in tests/conftest.py to identify testbed type. Could you please review it?

@lguohan
Copy link
Contributor

lguohan commented Mar 20, 2020

second, i like the mark introduced in the pytest.ini, for example bsl. Then, we can use that mark to filter which test to run instead of using this skip.

for example, another requirement is to run all tests that allowed to run in t0 testbed. how do we specify that?

…s defined in tests/conftest.py to identify testbed type.
@EmmaLin11 EmmaLin11 requested a review from lguohan March 25, 2020 01:14
def get_testbed_type(self, topo_name):
testbed_type = ['t0', 't1', 'ptf']
for val in testbed_type:
if topo_name.find(val) != -1:
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest to use regex match somehting like ("^(t0|t1|ptf)[-]")?

Copy link
Contributor Author

@EmmaLin11 EmmaLin11 Mar 26, 2020

Choose a reason for hiding this comment

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

@lguohan , thank you for your advice. I change to use regex match() to check if topo_name matches expected topo_type. Could you please review it?

@lguohan
Copy link
Contributor

lguohan commented Mar 26, 2020

once you change to use regex then, i am good.

@lguohan lguohan merged commit 2514341 into sonic-net:master Mar 26, 2020
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
9e740759c370645b4367acf22856aebcfb7fce45 (HEAD -> 201911, origin/201911) [201911][multi asic] show ip bgp summary changes for bgp mon (sonic-net#1483)
fa07245786df11e6df902c33fcd9c7115a7c5380 [CLI][techsupport] Merge 'show techsupport' changes from master (sonic-net#1468)

Signed-off-by: Abhishek Dosi <[email protected]>
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.

3 participants