Add tests for copp manager redesign#4891
Conversation
tests/copp/test_copp.py
Outdated
| logger.info("Config save") | ||
| duthost.command("sudo config save -y") | ||
|
|
||
| reboot_type = random.choice(["cold", "fast", "warm", "soft"]) |
There was a problem hiding this comment.
@JibinBao is choosing a random type of reboot intended to verify behavior in case of any reboot type?
If the idea was to check this for all 4 types of reboots, I would suggest using test parametrization. Or as another option reboot type could be passed as command line argument using parser.adoption (similar to --send_rate_limit parameter). Any of these 2 approaches would be useful if we want to test a specific type of reboot.
There was a problem hiding this comment.
Hi @vcheketx ,
Because reboot consumes time very much, so we just want to choosing one randomly for one time.
When we run the test for multiple times, all reboot-type will be covered.
So we don't use test parametrization and parser.adoption.
thanks
There was a problem hiding this comment.
If it doesn't work for one reboot type, then the test case would be flaky and cause trouble.
Can you add an option to cover all reboot types? If the option is not supplied, just use one fixed default reboot type.
There was a problem hiding this comment.
Or add an option for specifying reboot type as vscheketx suggested?
Add an option for specifing reboot type for copp reboot test, so make case result more stable
|
@wangxin could you please to review the fixes following your feedback? |
liat-grozovik
changed the title
|
@vcheketx any further comment or we can move forward and merge once the relevant code PR. |
@liat-grozovik @JibinBao no further comments, thanks for implementing option for specifying reboot type. |
What is the motivation for this PR? test_trap_config_save_after_reboot case should skipped in 202012 branch, since always_enable feature is not merged into 202012 image. [CoPP] Add always_enabled field to coppmgr logic by noaOrMlnx · Pull Request #2034 · sonic-net/sonic-swss (github.com) Test case was added in this PR: #4891 xfail in this PR [test] skip/xfail unmerged copp feature temporarily by yejianquan · Pull Request #5148 · sonic-net/sonic-mgmt (github.com) How did you do it? Xfail is not correct, it needs to be skipped for now. Change xfail to skip. Signed-off-by: Zhaohui Sun <[email protected]>
What is the motivation for this PR? test_trap_config_save_after_reboot case should skipped in 202012 branch, since always_enable feature is not merged into 202012 image. [CoPP] Add always_enabled field to coppmgr logic by noaOrMlnx · Pull Request #2034 · sonic-net/sonic-swss (github.com) Test case was added in this PR: #4891 xfail in this PR [test] skip/xfail unmerged copp feature temporarily by yejianquan · Pull Request #5148 · sonic-net/sonic-mgmt (github.com) How did you do it? Xfail is not correct, it needs to be skipped for now. Change xfail to skip. Signed-off-by: Zhaohui Sun <[email protected]>
…7373) What is the motivation for this PR? test_trap_config_save_after_reboot case should skipped in 202012 branch, since always_enable feature is not merged into 202012 image. [CoPP] Add always_enabled field to coppmgr logic by noaOrMlnx · Pull Request sonic-net#2034 · sonic-net/sonic-swss (github.com) Test case was added in this PR: sonic-net#4891 xfail in this PR [test] skip/xfail unmerged copp feature temporarily by yejianquan · Pull Request sonic-net#5148 · sonic-net/sonic-mgmt (github.com) How did you do it? Xfail is not correct, it needs to be skipped for now. Change xfail to skip. Signed-off-by: Zhaohui Sun <[email protected]>
|
Hi @JibinBao , why you not check |
5 participants
Description of PR
Add tests for CoPP Manager redesign feature
Summary:
Fixes # (issue)
Dependency: Only when the PR below is merged, this PR can be merged
sonic-net/sonic-swss#2034
Type of change
Back port request
Approach
What is the motivation for this PR?
Add new tests for CoPP Redesign feature.
How did you do it?
Add new tests
How did you verify/test it?
Run New CoPP tests
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation