Skip to content
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

[dualtor][non-functional] add test template for active-active cable type #5545

Merged
merged 4 commits into from
May 18, 2022

Conversation

zjswhhh
Copy link
Contributor

@zjswhhh zjswhhh commented Apr 20, 2022

Description of PR

Summary:
Fixes # (issue)

Submitting this PR to re-add test template for active-active cable type. The change (#5424) was reverted earlier due to regression.

sign-off: Jing Zhang zhangjing@microsoft.com

Type of change

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

Back port request

  • 201911
  • 202012

Approach

What is the motivation for this PR?

Re-add the roadmap for supporting active-active dualtor_io testcases.

How did you do it?

  1. Pick the commit from the original PR.
  2. Import cable_type fixture where the modified fixtures are requested.
  3. Update data_plane_utils.py to re-use DualtorIO object.

How did you verify/test it?

Tested test cases below, all passed:

dualtor_io/test_normal_op.py::test_normal_op_upstream[active-standby] 
dualtor_io/test_normal_op.py::test_normal_op_downstream_active[active-standby] 
dualtor_io/test_normal_op.py::test_normal_op_downstream_standby[active-standby] 

dualtor_io/test_link_failure.py::test_active_link_down_downstream_active[active-standby] 
dualtor_io/test_link_failure.py::test_active_link_down_downstream_standby[active-standby] 
dualtor_io/test_link_failure.py::test_standby_link_down_upstream[active-standby] 
dualtor_io/test_link_failure.py::test_standby_link_down_downstream_active[active-standby] 
dualtor_io/test_link_failure.py::test_standby_link_down_downstream_standby[active-standby] 
dualtor_io/test_link_failure.py::test_active_tor_downlink_down_upstream[active-standby] 
dualtor_io/test_link_failure.py::test_active_tor_downlink_down_downstream_active[active-standby] 
dualtor_io/test_link_failure.py::test_active_tor_downlink_down_downstream_standby[active-standby] 
dualtor_io/test_link_failure.py::test_standby_tor_downlink_down_upstream[active-standby] 
dualtor_io/test_link_failure.py::test_standby_tor_downlink_down_downstream_active[active-standby] 
dualtor_io/test_link_failure.py::test_standby_tor_downlink_down_downstream_standby[active-standby] 

Any platform specific information?

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

Documentation

lolyu and others added 2 commits April 20, 2022 15:07
…c-net#5424)

Approach
What is the motivation for this PR?
Modify testcase test_normal_op.py::test_normal_op_upstream to support parameterization based on cable type(active-active or active-standby).
This PR acts as a roadmap on how to support cable type parameterization further for all dualtor_io testcases.

Signed-off-by: Longxiang Lyu lolv@microsoft.com

How did you do it?
1. Add parameterization fixture cable_type.
2. Let existing fixtures for dualtor active-standby selects ports in active-standby cable type to test.
3. List out the basic APIs for active-active tests control/data plane fixtures.

How did you verify/test it?
Run the test test_normal_op.py::test_normal_op_upstream:
@zjswhhh zjswhhh requested a review from a team as a code owner April 20, 2022 23:50
@lgtm-com
Copy link

lgtm-com bot commented Apr 21, 2022

This pull request introduces 13 alerts when merging 9dfe02b into 3212c2f - view on LGTM.com

new alerts:

  • 13 for Unused import

@zjswhhh
Copy link
Contributor Author

zjswhhh commented Apr 21, 2022

/azp run Azure.sonic-mgmt

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@zjswhhh zjswhhh requested review from lolyu and yxieca April 22, 2022 21:14
Copy link
Contributor

@lolyu lolyu left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

lolyu
lolyu previously approved these changes Apr 25, 2022
@zjswhhh
Copy link
Contributor Author

zjswhhh commented Apr 26, 2022

/azp run Azure.sonic-mgmt

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

yxieca
yxieca previously approved these changes Apr 28, 2022
@yxieca
Copy link
Collaborator

yxieca commented Apr 28, 2022

@zjswhhh did you address the LGTM issues?

@wangxin
Copy link
Collaborator

wangxin commented Apr 29, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@zjswhhh zjswhhh dismissed stale reviews from yxieca and lolyu via ef53d71 May 5, 2022 23:26
@lgtm-com
Copy link

lgtm-com bot commented May 5, 2022

This pull request introduces 14 alerts when merging ef53d71 into fcc122b - view on LGTM.com

new alerts:

  • 14 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented May 6, 2022

This pull request introduces 14 alerts when merging 90520fa into 4dc8cfd - view on LGTM.com

new alerts:

  • 14 for Unused import

@zjswhhh zjswhhh requested review from yxieca and lolyu May 6, 2022 02:02
Copy link
Contributor

@lolyu lolyu left a comment

Choose a reason for hiding this comment

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

LGTM

@yxieca
Copy link
Collaborator

yxieca commented May 6, 2022

@zjswhhh are you able to solve the 14 unused imports identified by LGTM?

@zjswhhh
Copy link
Contributor Author

zjswhhh commented May 9, 2022

@zjswhhh are you able to solve the 14 unused imports identified by LGTM?

Hi Ying, the alerting for unused import is false positive. Let me check if I can find a work-around.

@zjswhhh zjswhhh merged commit e80445f into sonic-net:master May 18, 2022
@zjswhhh zjswhhh deleted the active_active_master branch May 18, 2022 17:51
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