-
Notifications
You must be signed in to change notification settings - Fork 731
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Modify tests to run on zero ports systems #3663
Conversation
Signed-off-by: Sharon Lutati <[email protected]>
This pull request introduces 3 alerts and fixes 2 when merging 39f18e6 into a26f2d5 - view on LGTM.com new alerts:
fixed alerts:
|
@slutati1536 please handle new LGTM errors |
@wangxin would you mind to help review the changes? |
Signed-off-by: Sharon Lutati <[email protected]>
Signed-off-by: Sharon Lutati <[email protected]>
This pull request fixes 2 alerts when merging 4d061f5 into 7b4a9e1 - view on LGTM.com fixed alerts:
|
Signed-off-by: Sharon Lutati <[email protected]>
This pull request fixes 2 alerts when merging b96e0f4 into e6741d3 - view on LGTM.com fixed alerts:
|
Signed-off-by: Sharon Lutati <[email protected]>
This pull request fixes 2 alerts when merging 447d6fc into ab82dd1 - view on LGTM.com fixed alerts:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slutati1536 please add for each test logic change the reason why it does not run when there are no interfaces and how the test behaves in this case: cont without failure, pass, ignore, etc.
Signed-off-by: Sharon Lutati <[email protected]>
This pull request fixes 2 alerts when merging 136d9b9 into 428e44a - view on LGTM.com fixed alerts:
|
Signed-off-by: Sharon Lutati <[email protected]>
This pull request fixes 2 alerts when merging ba0f3ec into 73c2d17 - view on LGTM.com fixed alerts:
|
Signed-off-by: Sharon Lutati <[email protected]>
Signed-off-by: Sharon Lutati <[email protected]>
This pull request fixes 2 alerts when merging b392c01 into 6cc2135 - view on LGTM.com fixed alerts:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except some minor comments. Can you resolve the merge conflict? @slutati1536
…nic/config_db.json
…nic/config_db.json
Hello All, I have a task in the works for handling this PR, which should be resolve by the end of agust. |
…removed by mistack- test shouldn't run on setup with no ports
This pull request introduces 1 alert and fixes 2 when merging bc97af9 into 741c735 - view on LGTM.com new alerts:
fixed alerts:
|
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@sujinmkang can you please help to review? |
resolve merge conflict of test_telemetry
This pull request introduces 1 alert and fixes 2 when merging a9f9453 into f50ae15 - view on LGTM.com new alerts:
fixed alerts:
|
Details: basing interfaces list on config_db (which is done by get_port_map) should be done in all cases not only where asic is not None. Also, added get_dev_conn to test_check_sfp_using_ethtool, as it is also used in that way in other sfp tests.
This pull request introduces 1 alert and fixes 2 when merging 998955e into b338a88 - view on LGTM.com new alerts:
fixed alerts:
|
This pull request fixes 2 alerts when merging 15ad6f1 into eefc0e5 - view on LGTM.com fixed alerts:
|
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
This pull request fixes 2 alerts when merging 4ce5ed0 into 171df86 - view on LGTM.com fixed alerts:
|
@liat-grozovik Currently merging of this PR is blocked by your change request. If you have no concern with this PR, can you approve it? Then we can proceed with merging. |
Allow systems which starts with zero ports to run platform tests without failing even if the tests skip the interface checks. Change the port verification to relay on config_db.json instead of minigraph.xml Change places where test fail in case of zero ports on setup What is the motivation for this PR? Allow systems which starts with zero ports to run platform tests without failing even if the tests skip the interface checks. How did you do it? Rerun pre defined list of tests where system is defined with zero ports and modify tests one by one to pass but to keep functional as possible (taking into account that interfaces are not configured) How did you verify/test it? Rerun tests and ensure they are passing on a system with ports and with zero ports. Signed-off-by: Sharon Lutati <[email protected]>
Signed-off-by: Sharon Lutati [email protected]
Description of PR
Allow systems which starts with zero ports to run platform tests without failing even if the tests skip the interface checks.
Summary:
Modify tests so that they will pass also on setup with zero ports
Type of change
Approach
What is the motivation for this PR?
Allow systems which starts with zero ports to run platform tests without failing even if the tests skip the interface checks.
How did you do it?
Rerun pre defined list of tests where system is defined with zero ports and modify tests one by one to pass but to keep functional as possible (taking into account that interfaces are not configured)
How did you verify/test it?
Rerun tests and ensure they are passing on a system with ports and with zero ports.
Any platform specific information?
No
Supported testbed topology if it's a new test case?
T0 and T1-lag pre defined list of tests