[tests/platform/test_reboot.py] add testcases for reboot cause by stephenxs · Pull Request #1079 · sonic-net/sonic-mgmt

stephenxs · 2019-08-23T05:27:01Z

Description of PR

Summary:
add testcases for reboot cause

Type of change

[] Bug fix
[] Testbed and Framework(new/improvement)
Test case(improvement)

Approach

How did you do it?

How did you do it?
add hardware reboot testcases to reboot test
add reboot-cause checking after each reboot.

How did you verify/test it?

run test on every testbed

Any platform specific information?

Supported testbed topology if it's a new test case?

all topo

Documentation

[sonic_platform_test_plan.md]add reboot cause test #451

…rting it.

tests/platform/test_reboot.py

…f reboot test

tests/platform/test_reboot.py

wangxin · 2019-09-19T03:12:00Z

tests/platform/test_reboot.py

+    assert m is not None, "got reboot-cause %s after rebooted by %s" % (reboot_cause_got, reboot_cause_expected)
+
+
+def reboot_and_check(localhost, dut, interfaces, reboot_type="cold", reboot_helper=None, reboot_argu=None):


Parameter passing should use pre-defined constant:

reboot_type=REBOOT_TYPE_COLD

Suggest to use a better name reboot_kwargs.

wangxin · 2019-09-19T03:12:04Z

tests/platform/test_reboot.py

+
+    reboot_ctrl_element = reboot_ctrl_dict.get(reboot_type)
+    if reboot_ctrl_element is None:
+        assert False, "Unknown reboot type %s" % reboot_type


The above 3 lines of code can be simplified in a more pythonic way:

assert reboot_type in REBOOT_TYPES.keys(), "Unknown reboot type %s" % reboot_type

wangxin · 2019-09-19T03:12:06Z

tests/platform/test_reboot.py

+        assert False, "Unknown reboot type %s" % reboot_type
+
+    reboot_timeout = reboot_ctrl_element[REBOOT_TIMEOUT]
+    reboot_cause = reboot_ctrl_element[REBOOT_CAUSE]


If we use a simplified REBOOT_TYPES dict, then the above two lines should be like below:

reboot_timeout = REBOOT_TYPES[reboot_type]["timeout"] reboot_cause = REBOOT_TYPES[reboot_type]["cause"]

tests/platform/test_reboot.py

wangxin · 2019-09-19T03:12:11Z

tests/platform/test_reboot.py

-            process.terminate()
-        logging.error("reboot result %s" % str(queue.get()))
-        assert False, "DUT did not go down"
+        reboot_cmd = reboot_ctrl_element[REBOOT_COMMAND]


reboot_cmd = REBOOT_TYPES[reboot_type]["command"]

wangxin · 2019-09-19T03:12:14Z

tests/platform/test_reboot.py

+    return request.param
+
+
+def _power_off_reboot_helper(args):


Since this function expects a dictionary argument, it usually a better practice to name the argument like kwargs.

wangxin · 2019-09-19T03:12:16Z

tests/platform/test_reboot.py

+
+    delay_time_list = [15, 5]
+    poweroff_reboot_argu = {}
+    poweroff_reboot_argu["dut"] = ans_host


Suggest to use a better variable name poweroff_reboot_kwargs.

wangxin · 2019-09-19T03:12:18Z

tests/platform/test_reboot.py

+    ans_host = testbed_devices["dut"]
+    localhost = testbed_devices["localhost"]
+
+    watchdog_reboot_command = "python -c \"import sonic_platform.platform as P; P.Platform().get_chassis().get_watchdog().arm(5); exit()\""


This watchdog_reboot_command variable is unnecessary here. It's already in the REBOOT_TYPES dict.

wangxin · 2019-09-19T03:12:20Z

tests/platform/test_reboot.py

+    watchdog_reboot_argu = {}
+    watchdog_reboot_argu["dut"] = ans_host
+    watchdog_reboot_argu["cause"] = "Watchdog"
+    watchdog_reboot_argu["command"] = watchdog_reboot_command


This watchdog_reboot_argu variable is not used. Why define it here?

jleveque

Please resolve confilcts.

stephenxs · 2019-09-20T00:38:55Z

Fixed.

* [201811][swss][utilities] advance sub module head Submodule src/sonic-swss fcd091c..9585be4: > [teamsyncd]: Check if LAG exists before removing (sonic-net#1069) > [mirrororch]: Toggle the mirror session when moving between VLAN/non-VLAN (sonic-net#1078) Submodule src/sonic-utilities 7bb6ffb..2d721de: > [show] Properly replace port name with alias in command output (sonic-net#664) > [neighbor_advertiser] hand pick partial change from sonic-net#525 (sonic-net#689) > Revert "Revert "Fixed config Asym PFC CLI. (sonic-net#632)" (sonic-net#652)" Signed-off-by: Ying Xie <[email protected]> * address compile issue Submodule src/sonic-swss 9585be4..2529d79: > [mirrororch]: Address compiler switch issue (sonic-net#1079) Signed-off-by: Ying Xie <[email protected]>

Stephen Sun added 2 commits August 23, 2019 08:20

[tests/platform/test_reboot.py] add testcases for reboot cause

122c681

[test_reboot.py]don't run watchdog reboot test on DUTs that not suppo…

01379e8

…rting it.

stephenxs marked this pull request as ready for review August 26, 2019 05:50

jleveque added the Enhancement label Sep 9, 2019

jleveque approved these changes Sep 9, 2019

View reviewed changes

wangxin mentioned this pull request Sep 10, 2019

[platform/reboot] Fix the reboot stuck issue #1104

Closed

1 task

wangxin approved these changes Sep 16, 2019

View reviewed changes

liat-grozovik requested changes Sep 16, 2019

View reviewed changes

tests/platform/test_reboot.py Outdated Show resolved Hide resolved

tests/platform/test_reboot.py Outdated Show resolved Hide resolved

[platform/test_reboot.py] Use dictionary to represent the arguments o…

8be28ad

…f reboot test

wangxin requested changes Sep 19, 2019

View reviewed changes

[test_reboot.py] address Xin's comments. simplify code.

7a14d1d

liat-grozovik approved these changes Sep 19, 2019

View reviewed changes

jleveque suggested changes Sep 19, 2019

View reviewed changes

Merge branch 'master' into pr2-reboot-cause

26d1b06

[tests/platform/test_reboot.py]address comments

6d9fccc

jleveque approved these changes Sep 20, 2019

View reviewed changes

jleveque merged commit a3f2458 into sonic-net:master Sep 20, 2019

stephenxs deleted the pr2-reboot-cause branch September 25, 2019 03:25

		assert m is not None, "got reboot-cause %s after rebooted by %s" % (reboot_cause_got, reboot_cause_expected)


		def reboot_and_check(localhost, dut, interfaces, reboot_type="cold", reboot_helper=None, reboot_argu=None):

Conversation

stephenxs commented Aug 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

Type of change

Approach

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jleveque left a comment

Choose a reason for hiding this comment

Uh oh!

stephenxs commented Sep 20, 2019 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

stephenxs commented Aug 23, 2019 •

edited

Loading