-
Notifications
You must be signed in to change notification settings - Fork 717
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
Add reboot-cause history into reboot tests #4209
Conversation
a60b8ff
to
0854333
Compare
This pull request fixes 1 alert when merging 0854333 into 8c8d3aa - view on LGTM.com fixed alerts:
|
I think this reboot-history check is better suited for test_reboot.py and not test_advanced_reboot. The former is for checking causes, and platform status, and the latter is for I/O related verification in dataplane and controlplane tests. The test_reboot cases are located here: https://github.com/Azure/sonic-mgmt/blob/master/tests/platform_tests/test_reboot.py For example, this present check can be enhanced to check reboot cause history: |
Also these 3 checks in the new function are all verifying cosmetic appearance of the output. For example, if the output contains specific number of dotted lines in line 2. I think it would be more useful:
|
r"Hardware .*|Power Loss .*)[\s]{2,}(N\/A|(Mon|Tue|Wed|Thu|Fri|Sat|Sun) .*)" | ||
r"[\s]{2,}(.*)[\s]{2,}(.*)" | ||
} | ||
stdout, stderr, return_code = self.dut_connection.execCommand("show reboot-cause history") |
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.
The above command output makes it suitable for use show_and_parse: https://github.com/Azure/sonic-mgmt/blob/master/tests/common/devices/sonic.py#L1139
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.
This is a PTF script. The show_and_parse
method is not available here.
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.
Thank you.
I will close the PR, and then add the function into sonic-mgmt/tests/platform_tests/test_reboot.py
It would be better to add a separate test and reboot 3 times with different reboot types (could be randomly selected from available list) and check reboot history step by step. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
thank you. |
Description of PR
Summary:
Fixes # (issue)
Since adding the feature(sonic-net/SONiC#669) of show reboot-cause history, so integrate "show reboot-cause history" into reboot tests.
e.g.
Type of change
Back port request
Approach
What is the motivation for this PR?
In order to check reboot-cause history function, integrate "show reboot-cause history" into reboot tests.
How did you do it?
Add one function of check_reboot_cause_history_after_reboot in ansible/roles/test/files/ptftests/advanced-reboot.py.
After doing reboot test, and then check the reboot cause history.
How did you verify/test it?
run fast-reboot or warm-reboot test case with ansible
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation
sonic-net/SONiC#669