tests/crm: Marvell specific changes#6994
Merged
Blueve merged 1 commit intosonic-net:masterfrom Dec 14, 2022
Merged
Conversation
3 tasks
Collaborator
|
Do we need this fix to 201911 and 202012? |
Contributor
Author
Blueve
reviewed
Dec 8, 2022
Blueve
reviewed
Dec 8, 2022
dfcf055 to
c851583
Compare
Blueve
approved these changes
Dec 12, 2022
Blueve
approved these changes
Dec 12, 2022
Collaborator
|
Hi @sureshsekar28, there are some new styling issues introduced by your change, can you help fix them? |
Blueve
requested changes
Dec 12, 2022
Collaborator
Blueve
left a comment
There was a problem hiding this comment.
Hi @sureshsekar28, there are some new styling issues introduced by your change, can you help fix them?
tests/crm/test_crm.py:555:121: E501 line too long (141 > 120 characters)
tests/crm/test_crm.py:561:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:561:40: E231 missing whitespace after ','
tests/crm/test_crm.py:562:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:562:45: E231 missing whitespace after ','
tests/crm/test_crm.py:563:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:564:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:565:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:566:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:568:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:568:40: E231 missing whitespace after ','
tests/crm/test_crm.py:569:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:569:45: E231 missing whitespace after ','
tests/crm/test_crm.py:570:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:571:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:572:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:573:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:574:53: E231 missing whitespace after ','
tests/crm/test_crm.py:574:53: E231 missing whitespace after ','
tests/crm/test_crm.py:610:51: E231 missing whitespace after ','
tests/crm/test_crm.py:610:63: E231 missing whitespace after ','
As per Marvell design nexthop increments only after adding route Signed-off-by: sureshsekar28 <sureshsekar@marvell.com> addressed review comments addressed whitespace styling issues
c851583 to
a34f7b2
Compare
Contributor
Author
|
Thanks. I have corrected styling issues. Please check now.
From: Jing Kan ***@***.***>
Date: Monday, 12 December 2022 at 2:01 PM
To: sonic-net/sonic-mgmt ***@***.***>
Cc: Suresh Sekar ***@***.***>, Mention ***@***.***>
Subject: [EXT] Re: [sonic-net/sonic-mgmt] tests/crm: Marvell specific changes (PR #6994)
External Email
…________________________________
@Blueve requested changes on this pull request.
Hi @sureshsekar28<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_sureshsekar28&d=DwMCaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=B-jTQXldORQCDMAUB6mwEwDb_1YTzZkwzSjZuec-4Y4&m=h5kuoOLnPo8IjK72yGDR77-GBmqIP8vuCHV5Vv0P2FoCKqzcD2cB-5E2mdlvCZQ_&s=51fdelZPX1e7xIFA3Ql8AkllMJ3yuGrbS-4FnowMy94&e=>, there are some new styling issues introduced by your change, can you help fix them?
tests/crm/test_crm.py:555:121: E501 line too long (141 > 120 characters)
tests/crm/test_crm.py:561:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:561:40: E231 missing whitespace after ','
tests/crm/test_crm.py:562:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:562:45: E231 missing whitespace after ','
tests/crm/test_crm.py:563:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:564:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:565:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:566:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:568:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:568:40: E231 missing whitespace after ','
tests/crm/test_crm.py:569:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:569:45: E231 missing whitespace after ','
tests/crm/test_crm.py:570:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:571:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:572:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:573:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:574:53: E231 missing whitespace after ','
tests/crm/test_crm.py:574:53: E231 missing whitespace after ','
tests/crm/test_crm.py:610:51: E231 missing whitespace after ','
tests/crm/test_crm.py:610:63: E231 missing whitespace after ','
—
Reply to this email directly, view it on GitHub<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_sonic-2Dnet_sonic-2Dmgmt_pull_6994-23pullrequestreview-2D1213084118&d=DwMCaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=B-jTQXldORQCDMAUB6mwEwDb_1YTzZkwzSjZuec-4Y4&m=h5kuoOLnPo8IjK72yGDR77-GBmqIP8vuCHV5Vv0P2FoCKqzcD2cB-5E2mdlvCZQ_&s=dzbVLjclq3kSKw4m7fZIZTZfYvfhwhOvZ1fZgGBgPzg&e=>, or unsubscribe<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AYWTPY7RWUZIGBGDZK22OPDWM3PF7ANCNFSM6AAAAAASXYZFHA&d=DwMCaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=B-jTQXldORQCDMAUB6mwEwDb_1YTzZkwzSjZuec-4Y4&m=h5kuoOLnPo8IjK72yGDR77-GBmqIP8vuCHV5Vv0P2FoCKqzcD2cB-5E2mdlvCZQ_&s=wmc4Xg_psWIaDpgKuTElQjIU0dfgrjiWotW6T52yMIk&e=>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Contributor
Author
|
I have addressed all the review comments. Please review.
Thanks,
Suresh.
From: Suresh Sekar ***@***.***>
Date: Monday, 12 December 2022 at 3:07 PM
To: sonic-net/sonic-mgmt ***@***.***>, sonic-net/sonic-mgmt ***@***.***>
Cc: Mention ***@***.***>
Subject: Re: [EXT] Re: [sonic-net/sonic-mgmt] tests/crm: Marvell specific changes (PR #6994)
Thanks. I have corrected styling issues. Please check now.
From: Jing Kan ***@***.***>
Date: Monday, 12 December 2022 at 2:01 PM
To: sonic-net/sonic-mgmt ***@***.***>
Cc: Suresh Sekar ***@***.***>, Mention ***@***.***>
Subject: [EXT] Re: [sonic-net/sonic-mgmt] tests/crm: Marvell specific changes (PR #6994)
External Email
…________________________________
@Blueve requested changes on this pull request.
Hi @sureshsekar28<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_sureshsekar28&d=DwMCaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=B-jTQXldORQCDMAUB6mwEwDb_1YTzZkwzSjZuec-4Y4&m=h5kuoOLnPo8IjK72yGDR77-GBmqIP8vuCHV5Vv0P2FoCKqzcD2cB-5E2mdlvCZQ_&s=51fdelZPX1e7xIFA3Ql8AkllMJ3yuGrbS-4FnowMy94&e=>, there are some new styling issues introduced by your change, can you help fix them?
tests/crm/test_crm.py:555:121: E501 line too long (141 > 120 characters)
tests/crm/test_crm.py:561:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:561:40: E231 missing whitespace after ','
tests/crm/test_crm.py:562:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:562:45: E231 missing whitespace after ','
tests/crm/test_crm.py:563:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:564:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:565:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:566:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:568:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:568:40: E231 missing whitespace after ','
tests/crm/test_crm.py:569:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:569:45: E231 missing whitespace after ','
tests/crm/test_crm.py:570:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:571:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:572:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:573:12: E111 indentation is not a multiple of 4
tests/crm/test_crm.py:574:53: E231 missing whitespace after ','
tests/crm/test_crm.py:574:53: E231 missing whitespace after ','
tests/crm/test_crm.py:610:51: E231 missing whitespace after ','
tests/crm/test_crm.py:610:63: E231 missing whitespace after ','
—
Reply to this email directly, view it on GitHub<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_sonic-2Dnet_sonic-2Dmgmt_pull_6994-23pullrequestreview-2D1213084118&d=DwMCaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=B-jTQXldORQCDMAUB6mwEwDb_1YTzZkwzSjZuec-4Y4&m=h5kuoOLnPo8IjK72yGDR77-GBmqIP8vuCHV5Vv0P2FoCKqzcD2cB-5E2mdlvCZQ_&s=dzbVLjclq3kSKw4m7fZIZTZfYvfhwhOvZ1fZgGBgPzg&e=>, or unsubscribe<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AYWTPY7RWUZIGBGDZK22OPDWM3PF7ANCNFSM6AAAAAASXYZFHA&d=DwMCaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=B-jTQXldORQCDMAUB6mwEwDb_1YTzZkwzSjZuec-4Y4&m=h5kuoOLnPo8IjK72yGDR77-GBmqIP8vuCHV5Vv0P2FoCKqzcD2cB-5E2mdlvCZQ_&s=wmc4Xg_psWIaDpgKuTElQjIU0dfgrjiWotW6T52yMIk&e=>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Blueve
approved these changes
Dec 14, 2022
wangxin
pushed a commit
that referenced
this pull request
Dec 19, 2022
What is the motivation for this PR? As per Marvell design nexthop increments only after adding route How did you do it? script enhancement How did you verify/test it? script passing with the change Any platform specific information? Marvell specific changes Signed-off-by: sureshsekar28 <sureshsekar@marvell.com>
6 tasks
neethajohn
added a commit
that referenced
this pull request
Jan 20, 2023
Signed-off-by: Neetha John <nejo@microsoft.com> What is the motivation for this PR? The following commit done as part of cherry-picking to 202012 appears bad (ab3d07d) This was trying to bring in changes from #6968 and #6994 How did you do it? Cleaned up the inadvertent changes How did you verify/test it? Ran the crm testcase "test_crm_nexthop" with the changes and it passes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As per Marvell design nexthop increments only after adding route
Signed-off-by: sureshsekar28 sureshsekar@marvell.com
Description of PR
As per Marvell design nexthop increments only after adding route
Summary:
Fixes # As per Marvell design nexthop increments only after adding route
Type of change
Back port request
Approach
What is the motivation for this PR?
As per Marvell design nexthop increments only after adding route
How did you do it?
script enhancement
How did you verify/test it?
script passing with the change
Any platform specific information?
Marvell specific changes
Supported testbed topology if it's a new test case?
Documentation
NA