-
Notifications
You must be signed in to change notification settings - Fork 771
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
Changes to support asic name change from marvell to marvell-prestera #17171
base: master
Are you sure you want to change the base?
Changes to support asic name change from marvell to marvell-prestera #17171
Conversation
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Commenter does not have sufficient privileges for PR 17171 in repo sonic-net/sonic-mgmt |
/azp run |
Commenter does not have sufficient privileges for PR 17171 in repo sonic-net/sonic-mgmt |
/azp run |
Commenter does not have sufficient privileges for PR 17171 in repo sonic-net/sonic-mgmt |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@@ -880,7 +880,7 @@ platform_tests/broadcom: | |||
skip: | |||
reason: "Marvell devices does not support platform_tests/broadcom" | |||
conditions: | |||
- "'marvell' in asic_type" | |||
- "'marvell' in asic_type or 'marvell-prestera' in asic_type" |
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.
"asic_type in ['marvell', 'marvell-prestera']" is the deterministic way?
the check above will include marvel-*
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.
Hi @yxieca
Marvell ASIC name is changed from “marvell” to “marvell-prestera” however we also have “marvell-teralynx”. If we use “marvell-*”, test cases will fail for marvell teralynx platforms
Thanks
Rajneesh
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.
That is the point. right?
'marvell' in asic_type already covered 'marvell-prestera' in asic_type" and it will pass return true for marvell-teralynx.
If your intention is to allow test on all 3 platforms, keeping the first condition is sufficient.
tests/common/plugins/conditional_mark/tests_mark_conditions_platform_tests.yaml
Outdated
Show resolved
Hide resolved
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Commenter does not have sufficient privileges for PR 17171 in repo sonic-net/sonic-mgmt |
Description of PR
Adding the changes to support chip name change from marvell to marvell-prestera
Summary:
Fixes # (issue)
The chip name for marvell is changed to marvell-prestera
Same need to be modified in sonic-mgmt where we do conditional checks for different scripts.
Type of change
Back port request
Approach
What is the motivation for this PR?
How did you do it?
How did you verify/test it?
Any platform specific information?
sonic-net/sonic-buildimage#19829
Supported testbed topology if it's a new test case?