Modified test cases by QOS DB reference format remove#3824
Modified test cases by QOS DB reference format remove#3824smaheshm merged 3 commits intosonic-net:masterfrom
Conversation
|
This pull request introduces 1 alert when merging a5e6a86df1ae9ad301312a78d758c858cde3194a into 8f3ae4f - view on LGTM.com new alerts:
|
|
@AshokDaparthi please explain how this change was tested? Why would LGTM catch syntax errors? |
This code testing needs others repo changes, At present even i mentioned "Depends on" at present pull request "Checks/regression tests" not using new packages. I am looking at how to run sonic-mgmt tests. |
stephenxs
left a comment
There was a problem hiding this comment.
Approved for test_buffer (pytest). Please check with other reviewers as well.
Thanks.
|
HI @yxieca @lguohan @AshokDaparthi @neethajohn
|
83c38f4 to
e80769d
Compare
|
Build failure need be handled, this PR blocks sonic-net/sonic-buildimage#7375 |
7d417cb to
7eeac59
Compare
Syntax error fix Addressed review comment
7eeac59 to
3038adf
Compare
|
This pull request introduces 1 alert when merging 3038adf34405243a2f2a612c76a2bccca8b823f5 into 79e5e78 - view on LGTM.com new alerts:
|
3038adf to
2ef9e81
Compare
|
@stephenxs , @neethajohn - Could you please review this, Modified test cases to handle release. |
|
@AshokDaparthi , there is a lot of code change here. I am unable to tell for sure if all the relevant files have been modified. What is the testing that you have done? Can you share the test results? |
|
@neethajohn - I depend on "Checks" in pull requests. I tried simulating "present/master" release as new format and also tested "present/master" as old format. Both got passed. We do not have any simulated setup which can run sonic-mgmt qos test cases with h/w or vs. I did manual testing in broadcom platform. |
There was a problem hiding this comment.
I don't see any version check for all the files changed under the spytest folder. Am I missing something?
There was a problem hiding this comment.
@neethajohn - spytest are not handled for backward compatibility, i hope spytest are not used actively for sanity or test. I will take this up after all changes merged.
There was a problem hiding this comment.
@ramakristipati , please review the spytest changes
|
@stephenxs , there are some new changes. Can you please review again? |
fcd4365 to
b935879
Compare
b935879 to
03f4560
Compare
stephenxs
left a comment
There was a problem hiding this comment.
@AshokDaparthi Some minor comments.
Description of PR Qos sai failures seen due to changes in #3824 Signed-off-by: Neetha John <nejo@microsoft.com>
Depends on sonic-net/sonic-utilities#1626
Depends on sonic-net/sonic-swss#1754
Depends on sonic-net/sonic-buildimage#7752
QOS tables in config db used ABNF format i.e "[TABLE_NAME|name] to refer fieldvalue to other qos tables.
Example:
Config DB:
"Ethernet92|3": {
"scheduler": "[SCHEDULER|scheduler.1]",
"wred_profile": "[WRED_PROFILE|AZURE_LOSSLESS]"
},
"Ethernet0|0": {
"profile": "[BUFFER_PROFILE|ingress_lossy_profile]"
},
"Ethernet0": {
"dscp_to_tc_map": "[DSCP_TO_TC_MAP|AZURE]",
"pfc_enable": "3,4",
"pfc_to_queue_map": "[MAP_PFC_PRIORITY_TO_QUEUE|AZURE]",
"tc_to_pg_map": "[TC_TO_PRIORITY_GROUP_MAP|AZURE]",
"tc_to_queue_map": "[TC_TO_QUEUE_MAP|AZURE]"
},
This format is not consistent with other DB schema followed in sonic.
And also this reference in DB is not required, This is taken care by YANG "leafref".
Removed this format from all platform files to consistent with other sonic db schema.
Example:
"Ethernet92|3": {
"scheduler": "scheduler.1",
"wred_profile": "AZURE_LOSSLESS"
},
Modified test cases to remove DB references.