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.

Sign up for GitHub

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

Jump to bottom

[GCU] Interface Replace Lanes test to expect op success #5859

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[GCU] Interface Replace Lanes test to expect op success #5859

wants to merge 1 commit into from

Conversation

isabelmsft
Copy link
Contributor

Description of PR

Summary:
Fixes # (issue)

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?

Previously, the GCU eth interface replace lanes test expected op failure. However, based on SONiC YANG models, the lanes should be replaced successfully.
The original test passed (expect op failure) because of this issue that has now been fixed by this PR

How did you do it?

Change to expect op success

How did you verify/test it?

Any platform specific information?

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

Documentation

@isabelmsft isabelmsft requested a review from a team as a code owner June 22, 2022 19:35
@lgtm-com
Copy link

lgtm-com bot commented Jun 22, 2022

This pull request introduces 1 alert when merging fc69fe93d6b8769e4e8737a5d39d75a80f90a956 into a7f3015 - view on LGTM.com

new alerts:

  • 1 for Wrong number of arguments in a call

@qiluo-msft
Copy link
Contributor

qiluo-msft commented Jun 22, 2022 •
edited
Loading 

update_lanes = cur_lanes

Let's change the test case target config:
Ethernet0.lanes = [0,1,2,7]
Ethernet4.lanes = [4,5,6,3]

So it is expected succeed, even we add new enforcement of lane uniqueness.

In reply to: 1163555657

Refers to: tests/generic_config_updater/test_eth_interface.py:101 in fc69fe9. [](commit_id = fc69fe93d6b8769e4e8737a5d39d75a80f90a956, deletion_comment = False)

qiluo-msft
qiluo-msft previously approved these changes Jun 23, 2022
View reviewed changes
@ghooo
Copy link

ghooo commented Jun 24, 2022

The test_replace_lanes is failing during build.

Error from logs:

 "stderr": "Failed to apply patch\nUsage: config apply-patch [OPTIONS] PATCH_FILE_PATH\nTry \"config apply-patch -h\" for help.\n\nError: 'CABLE_LENGTH'"

This error was fixed by type1-list PR i.e. sonic-net/sonic-utilities#2171

Sonic-utils was recently updated (2days ago) in sonic-buildimage branch sonic-net/sonic-buildimage#11177

Can we check if kvmtest-t0-part2 includes this recent update?

@isabelmsft
Copy link
Contributor Author

For replace lanes test, opened new issue sonic-net/sonic-utilities#2263

@wangxin
Copy link
Collaborator

wangxin commented Jul 26, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@isabelmsft
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@isabelmsft
Squashed commit of the following:
3cef9af 
commit 16582f32c14d6b634e17702bb0e0d15250482016
Author: Ubuntu <isl@isl-dev-6.gccxvha3jmperpu0bogtupfduf.jx.internal.cloudapp.net>
Date:   Thu Jun 23 18:23:41 2022 +0000

    add parameter

commit 684ee051b91bb9ba1fa9adedef4abcbc8354a5c6
Author: Ubuntu <isl@isl-dev-6.gccxvha3jmperpu0bogtupfduf.jx.internal.cloudapp.net>
Date:   Thu Jun 23 18:15:45 2022 +0000

    lane swap

commit fc69fe93d6b8769e4e8737a5d39d75a80f90a956
Author: Ubuntu <isl@isl-dev-6.gccxvha3jmperpu0bogtupfduf.jx.internal.cloudapp.net>
Date:   Wed Jun 22 19:24:34 2022 +0000

    expect success for replaces lanes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wen587 wen587 wen587 left review comments

@qiluo-msft qiluo-msft qiluo-msft left review comments

@ghooo ghooo Awaiting requested review from ghooo

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@isabelmsft @qiluo-msft @ghooo @wangxin @wen587