Skip to content

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

Closed
isabelmsft wants to merge 1 commit intosonic-net:masterfrom
isabelmsft:gcu_replace_lanes_fix
Closed

[GCU] Interface Replace Lanes test to expect op success#5859
isabelmsft wants to merge 1 commit intosonic-net:masterfrom
isabelmsft:gcu_replace_lanes_fix

Conversation

@isabelmsft
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
Contributor

qiluo-msft commented Jun 22, 2022

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)

Comment thread tests/generic_config_updater/test_eth_interface.py Outdated
qiluo-msft
qiluo-msft previously approved these changes Jun 23, 2022
@ghooo
Copy link
Copy Markdown

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
Copy Markdown
Contributor Author

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

@wangxin
Copy link
Copy Markdown
Collaborator

wangxin commented Jul 26, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@isabelmsft
Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

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
@wangxinbot
Copy link
Copy Markdown
Contributor

This PR has been open since June 2022 (3+ years). LGTM flagged a "wrong number of arguments" alert, and test failures during build (CABLE_LENGTH error) were never resolved. The GCU test infrastructure has likely evolved since then. Recommend closing this PR.

@wangxin
Copy link
Copy Markdown
Collaborator

wangxin commented Apr 9, 2026

#5795

@wangxin wangxin closed this Apr 9, 2026
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.

6 participants