[arp] Allow use of discontiguous ports with ARP tests. #3776
Conversation
tests/arp/conftest.py
Outdated
| intf1 = ports[0] | ||
| intf2 = ports[1] | ||
| # Select first 2 ports that are admin 'up' | ||
| if duthost.is_multi_asic: |
There was a problem hiding this comment.
Better to avoid using "is_multi_asic" in the test logic. Use 'sonic.py', or 'sonic_asic.py' to add new APIs if required.
There was a problem hiding this comment.
Addressed in the latest commit
|
can you use https://github.com/Azure/sonic-mgmt/blob/8f37c4a6265eff229acf11165b976cdf3962991a/tests/common/devices/sonic_asic.py#L271 to get the active interfaces. |
tests/arp/conftest.py
Outdated
| # Select port index 0 & 1 two interfaces for testing | ||
| intf1 = ports[0] | ||
| intf2 = ports[1] | ||
| # Select first 2 ports that are admin 'up' |
There was a problem hiding this comment.
Why would testbed yaml file include ports that are admin down? If all the ports in testbed file are admin up, we don't need the change in the test, right?
There was a problem hiding this comment.
yaml file contains the testbed topology information. Sometimes the links are not up, not necessarily because of 'admin' state. It's good to get working ports for testing to reduce flakiness.
…- tcusto:arp_discont)
|
get_active_ip_interfaces returns the port channel interfaces as well. If we use get_active_ip_interfaces, we would have to add logic to differentiate between PortChannel and port IP interfaces, and if PortChannel, use its members. This would result in much more code changes. |
|
@smaheshm have incorporated your review comments. Could you please review again and help get this PR merged - Thanks. |
…- tcusto:arp_discont)
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Description of PR
Summary:
Allow use of discontiguous ports with ARP tests.
Type of change
Back port request
Approach
What is the motivation for this PR?
This change allows the ARP tests to run when not all front panel ports are connected to the fanout. Support was added for this configuration in PR 2517.
How did you do it?
Instead of picking first two ports on the card, do a show interface and find the first two ports that are admin up. Then proceed with the existing logic to move the port out of port channel if it is present.
How did you verify/test it?
Ran test_arpall.py on a card with non-adjacent ports.
15:22:54 conftest.intfs_for_test L0093 INFO | Selected ints are Ethernet1 and Ethernet11
=============================== warnings summary ===============================
/usr/local/lib/python2.7/dist-packages/_pytest/config/init.py:538
/usr/local/lib/python2.7/dist-packages/_pytest/config/init.py:538: PytestAssertRewriteWarning: Module already imported so cannot be rewritten: tests.common.dualtor
self.import_plugin(import_spec)
-- Docs: https://docs.pytest.org/en/latest/warnings.html
==================== 5 passed, 1 warnings in 175.22 seconds ====================
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation