From 16dba8327b3a737c09b1055bb3f974e666e9f4c6 Mon Sep 17 00:00:00 2001 From: sujinmkang Date: Fri, 10 Jul 2020 18:12:02 -0700 Subject: [PATCH 01/11] Changed to use PlatformApiTestBase to report the test failure. --- tests/platform_tests/api/test_watchdog.py | 112 +++++++++++----------- 1 file changed, 57 insertions(+), 55 deletions(-) diff --git a/tests/platform_tests/api/test_watchdog.py b/tests/platform_tests/api/test_watchdog.py index 36dfe1afc0..96f2e308d4 100644 --- a/tests/platform_tests/api/test_watchdog.py +++ b/tests/platform_tests/api/test_watchdog.py @@ -3,7 +3,9 @@ import logging import yaml import pytest -from tests.common.helpers.platform_api import watchdog +from common.helpers.platform_api import watchdog +from common.helpers.assertions import pytest_assert +from platform_api_test_base import PlatformApiTestBase pytestmark = [ pytest.mark.topology('any') @@ -15,7 +17,8 @@ TEST_WAIT_TIME_SECONDS = 2 TIMEOUT_DEVIATION = 2 -class TestWatchdogApi(object): + +class TestWatchdogApi(PlatformApiTestBase): ''' Hardware watchdog platform API test cases ''' @pytest.fixture(scope='function', autouse=True) @@ -47,17 +50,15 @@ def conf(self, request, duthost): if platform in test_config and 'default' in test_config[platform]: config.update(test_config[platform]['default']) - if platform in test_config and hwsku in test_config[platform]: config.update(test_config[platform][hwsku]) - assert 'valid_timeout' in config + self.expect('valid_timeout' in config, "valid_timeout is not defined in config") # make sure watchdog won't reboot the system when test sleeps for @TEST_WAIT_TIME_SECONDS - assert config['valid_timeout'] > TEST_WAIT_TIME_SECONDS * 2 - + self.expect(config['valid_timeout'] > TEST_WAIT_TIME_SECONDS * 2, "valid_timeout {} is too short".format(config['valid_timeout'])) + self.assert_expectations() return config - def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): ''' arm watchdog with a valid timeout value, verify it is in armed state, disarm watchdog and verify it is in disarmed state @@ -65,19 +66,33 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): watchdog_timeout = conf['valid_timeout'] actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) - assert actual_timeout != -1 - assert actual_timeout >= watchdog_timeout - assert watchdog.is_armed(platform_api_conn) + if self.expect(actual_timeout is not None, "Failed to arm the watchdog"): + self.expect(actual_timeout >= watchdog_timeout, "Actual watchdog setting with {} apears wrong from the original setting {}".format(actual_timeout, watchdog_timeout)) - assert watchdog.disarm(platform_api_conn) - assert not watchdog.is_armed(platform_api_conn) + watchdog_status = watchdog.is_armed(platform_api_conn) + if self.expect(watchdog_status is not None, "Failed to check the watchdog status"): + self.expect(watchdog_status is True, "Watchdog armed is expected but not armed.") + + remaining_time = watchdog.get_remaining_time(platform_api_conn) + if self.expect(remaining_time is not None, "Failed to get the remaining time of watchdog"): + self.expect(remaining_time <= watchdog_timeout, "watchdog remaining_time is not expected value {}".format(remaining_time)) + + watchdog_status = watchdog.disarm(platform_api_conn) + if self.expect(watchdog_status is not None, "Failed to disarm the watchdog"): + self.expect(watchdog_status is True, "Watchdog disarm returns False") + + watchdog_status = watchdog.is_armed(platform_api_conn) + if self.expect(watchdog_status is not None, "Failed to check the watchdog status"): + self.expect(watchdog_status is False, "Watchdog disarmed is expected but armed") - res = localhost.wait_for(host=duthost.hostname, - port=22, state="stopped", delay=5, - timeout=watchdog_timeout + TIMEOUT_DEVIATION, - module_ignore_errors=True) + remaining_time = watchdog.get_remaining_time(platform_api_conn) + if self.expect(remaining_time is not None, "Failed to get the remaining time of watchdog"): + self.expect(remaining_time is -1, "watchdog remaining_time is not expected value {}".format(remaining_time)) + + res = localhost.wait_for(host=duthost.hostname, port=22, state="stopped", delay=5, timeout=watchdog_timeout + TIMEOUT_DEVIATION, module_ignore_errors=True) - assert 'exception' in res + self.expect('exception' in res, "unexpected disconnection from dut") + self.assert_expectations() def test_remaining_time(self, duthost, platform_api_conn, conf): ''' arm watchdog with a valid timeout and verify that remaining time API works correctly ''' @@ -86,17 +101,20 @@ def test_remaining_time(self, duthost, platform_api_conn, conf): # in the begginging of the test watchdog is not armed, so # get_remaining_time has to return -1 - assert watchdog.get_remaining_time(platform_api_conn) == -1 - - actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) remaining_time = watchdog.get_remaining_time(platform_api_conn) + if self.expect(remaining_time is not None and remaining_time is -1, "watchdog should be disabled in the initial state"): + actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) + remaining_time = watchdog.get_remaining_time(platform_api_conn) - assert remaining_time > 0 - assert remaining_time <= actual_timeout + if self.expect(actual_timeout >= watchdog_timeout, "watchdog arm with {} seconds failed".format(watchdog_timeout)): + if self.expect(remaining_time > 0, "watchdog remaining_time {} is not valid".format(remaining_time)): + self.expect(remaining_time <= actual_timeout, "remaining_time {} should be less than watchdog armed timeout {}".format(remaining_timeout, actual_timeout)) remaining_time = watchdog.get_remaining_time(platform_api_conn) time.sleep(TEST_WAIT_TIME_SECONDS) - assert watchdog.get_remaining_time(platform_api_conn) < remaining_time + remaining_time_new = watchdog.get_remaining_time(platform_api_conn) + self.expect(remaining_time_new < remaining_time, "remaining_time {} should be decreased from previous remaining_time {}".format(remaining_time_new, remaining_time)) + self.assert_expectations() def test_periodic_arm(self, duthost, platform_api_conn, conf): ''' arm watchdog several times as watchdog deamon would and verify API behaves correctly ''' @@ -106,9 +124,11 @@ def test_periodic_arm(self, duthost, platform_api_conn, conf): time.sleep(TEST_WAIT_TIME_SECONDS) remaining_time = watchdog.get_remaining_time(platform_api_conn) actual_timeout_new = watchdog.arm(platform_api_conn, watchdog_timeout) + remaining_time_new = watchdog.get_remaining_time(platform_api_conn) - assert actual_timeout == actual_timeout_new - assert watchdog.get_remaining_time(platform_api_conn) > remaining_time + self.expect(actual_timeout == actual_timeout_new, "{}: new watchdog timeout {} setting should be same as the previous actual watchdog timeout {}".format(test_periodic_arm.__name__, actual_timeout_new, actual_timeout)) + self.expect(remaining_time_new > remaining_time, "{}: new remaining timeout {} should be bigger than the previous remaining timeout {} by {}".format(test_periodic_arm.__name__, remaining_time_new, remaining_time, TEST_WAIT_TIME_SECONDS)) + self.assert_expectations() def test_arm_different_timeout_greater(self, duthost, platform_api_conn, conf): ''' arm the watchdog with greater timeout value and verify new timeout was accepted; @@ -122,9 +142,10 @@ def test_arm_different_timeout_greater(self, duthost, platform_api_conn, conf): actual_timeout_second = watchdog.arm(platform_api_conn, watchdog_timeout) remaining_time = watchdog.get_remaining_time(platform_api_conn) actual_timeout_second_second = watchdog.arm(platform_api_conn, watchdog_timeout_greater) - - assert actual_timeout_second < actual_timeout_second_second - assert watchdog.get_remaining_time(platform_api_conn) > remaining_time + self.expect(actual_timeout_second < actual_timeout_second_second, "{}: 1st timeout {} should be smaller than 2nd timeout {}".format(test_arm_different_timeout_greater.__name__, actual_timeout_second, actual_timeout_second_second)) + remaining_time_second = watchdog.get_remaining_time(platform_api_conn) + self.expect(remaining_time_second > remaining_time, "{}: 2nd remaining_timeout {} should be bigger than 1st remaining timeout {}".format(test_arm_different_timeout_greater.__name__, remaining_time_second, remaining_time)) + self.assert_expectations() def test_arm_different_timeout_smaller(self, duthost, platform_api_conn, conf): ''' arm the watchdog with smaller timeout value and verify new timeout was accepted; @@ -139,8 +160,10 @@ def test_arm_different_timeout_smaller(self, duthost, platform_api_conn, conf): remaining_time = watchdog.get_remaining_time(platform_api_conn) actual_timeout_smaller = watchdog.arm(platform_api_conn, watchdog_timeout_smaller) - assert actual_timeout > actual_timeout_smaller - assert watchdog.get_remaining_time(platform_api_conn) < remaining_time + self.expect(actual_timeout > actual_timeout_smaller, "{}: 1st timeout {} should be bigger than 2nd timeout {}".format(test_arm_different_timeout_smaller.__name__, actual_timeout, actual_timeout_smaller)) + remaining_time_smaller = watchdog.get_remaining_time(platform_api_conn) + self.expect(remaining_time_smaller < remaining_time, "{}: 2nd remaining_timeout {} should be smaller than 1st remaining timeout {}".format(test_arm_different_timeout_smaller.__name__, remaining_time_smaller, remaining_time)) + self.assert_expectations() def test_arm_too_big_timeout(self, duthost, platform_api_conn, conf): ''' try to arm the watchdog with timeout that is too big for hardware watchdog; @@ -151,34 +174,13 @@ def test_arm_too_big_timeout(self, duthost, platform_api_conn, conf): if watchdog_timeout is None: pytest.skip('"too_big_timeout" parameter is required for this test case') actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) - - assert actual_timeout == -1 + self.expect(actual_timeout == -1, "{}: watchdog time {} shouldn't be set".format(test_arm_too_big_timeout.__name__, watchdog_timeout)) + self.assert_expectations() def test_arm_negative_timeout(self, duthost, platform_api_conn): ''' try to arm the watchdog with negative value ''' watchdog_timeout = -1 actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) - - assert actual_timeout == -1 - - @pytest.mark.disable_loganalyzer - def test_reboot(self, duthost, localhost, platform_api_conn, conf): - ''' arm the watchdog and verify it did its job after timeout expiration ''' - - watchdog_timeout = conf['valid_timeout'] - actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) - - assert actual_timeout != -1 - - res = localhost.wait_for(host=duthost.hostname, port=22, state="stopped", delay=2, - timeout=actual_timeout + TIMEOUT_DEVIATION, - module_ignore_errors=True) - assert 'exception' not in res - - res = localhost.wait_for(host=duthost.hostname, port=22, state="started", delay=10, timeout=120, - module_ignore_errors=True) - assert 'exception' not in res - - # wait for system to startup - time.sleep(120) + self.expect(actual_timeout == -1, "{}: watchdog time {} shouldn't be set".format(test_arm_negative_timeout.__name__, watchdog_timeout)) + self.assert_expectations() From 2c27b13287ee742f21437d885f3702ce04853042 Mon Sep 17 00:00:00 2001 From: sujinmkang Date: Mon, 13 Jul 2020 11:19:55 -0700 Subject: [PATCH 02/11] Added dependency and review commit --- tests/platform_tests/api/test_watchdog.py | 56 +++++++++++++---------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/tests/platform_tests/api/test_watchdog.py b/tests/platform_tests/api/test_watchdog.py index 96f2e308d4..41572fee89 100644 --- a/tests/platform_tests/api/test_watchdog.py +++ b/tests/platform_tests/api/test_watchdog.py @@ -55,10 +55,11 @@ def conf(self, request, duthost): self.expect('valid_timeout' in config, "valid_timeout is not defined in config") # make sure watchdog won't reboot the system when test sleeps for @TEST_WAIT_TIME_SECONDS - self.expect(config['valid_timeout'] > TEST_WAIT_TIME_SECONDS * 2, "valid_timeout {} is too short".format(config['valid_timeout'])) + self.expect(config['valid_timeout'] > TEST_WAIT_TIME_SECONDS * 2, "valid_timeout {} seconds is too short".format(config['valid_timeout'])) self.assert_expectations() return config + @pytest.mark.dependency() def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): ''' arm watchdog with a valid timeout value, verify it is in armed state, disarm watchdog and verify it is in disarmed state @@ -66,34 +67,36 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): watchdog_timeout = conf['valid_timeout'] actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) - if self.expect(actual_timeout is not None, "Failed to arm the watchdog"): - self.expect(actual_timeout >= watchdog_timeout, "Actual watchdog setting with {} apears wrong from the original setting {}".format(actual_timeout, watchdog_timeout)) + if self.expect(actual_timeout is not None, "Watchdog.arm is not supported"): + if self.expect(actual_timeout != -1, "Failed to arm the watchdog"): + self.expect(actual_timeout >= watchdog_timeout, "Actual watchdog {} seconds apears wrong, should be less than {} seconds".format(actual_timeout, watchdog_timeout)) watchdog_status = watchdog.is_armed(platform_api_conn) - if self.expect(watchdog_status is not None, "Failed to check the watchdog status"): - self.expect(watchdog_status is True, "Watchdog armed is expected but not armed.") + if self.expect(watchdog_status is not None, "Failed to retrieve watchdog status"): + self.expect(watchdog_status is True, "Watchdog is not armed.") remaining_time = watchdog.get_remaining_time(platform_api_conn) if self.expect(remaining_time is not None, "Failed to get the remaining time of watchdog"): - self.expect(remaining_time <= watchdog_timeout, "watchdog remaining_time is not expected value {}".format(remaining_time)) + self.expect(remaining_time <= watchdog_timeout, "Watchdog remaining_time {} seconds is wrong compared to watchdog timeout {} seocnds".format(remaining_time)) watchdog_status = watchdog.disarm(platform_api_conn) - if self.expect(watchdog_status is not None, "Failed to disarm the watchdog"): - self.expect(watchdog_status is True, "Watchdog disarm returns False") + if self.expect(watchdog_status is not None, "Watchdog.disarm is not supported"): + self.expect(watchdog_status is True, "Failed to disarm the watchdog") watchdog_status = watchdog.is_armed(platform_api_conn) if self.expect(watchdog_status is not None, "Failed to check the watchdog status"): - self.expect(watchdog_status is False, "Watchdog disarmed is expected but armed") + self.expect(watchdog_status is False, "Watchdog is not disarmed") remaining_time = watchdog.get_remaining_time(platform_api_conn) if self.expect(remaining_time is not None, "Failed to get the remaining time of watchdog"): - self.expect(remaining_time is -1, "watchdog remaining_time is not expected value {}".format(remaining_time)) + self.expect(remaining_time is -1, "Watchdog remaining_time {} seconds is wrong for disarmed state".format(remaining_time)) res = localhost.wait_for(host=duthost.hostname, port=22, state="stopped", delay=5, timeout=watchdog_timeout + TIMEOUT_DEVIATION, module_ignore_errors=True) self.expect('exception' in res, "unexpected disconnection from dut") self.assert_expectations() + @pytest.mark.dependency(depends=["test_arm_disarm_states"]) def test_remaining_time(self, duthost, platform_api_conn, conf): ''' arm watchdog with a valid timeout and verify that remaining time API works correctly ''' @@ -107,15 +110,16 @@ def test_remaining_time(self, duthost, platform_api_conn, conf): remaining_time = watchdog.get_remaining_time(platform_api_conn) if self.expect(actual_timeout >= watchdog_timeout, "watchdog arm with {} seconds failed".format(watchdog_timeout)): - if self.expect(remaining_time > 0, "watchdog remaining_time {} is not valid".format(remaining_time)): - self.expect(remaining_time <= actual_timeout, "remaining_time {} should be less than watchdog armed timeout {}".format(remaining_timeout, actual_timeout)) + if self.expect(remaining_time > 0, "Remaining_time {} seconds is not valid".format(remaining_time)): + self.expect(remaining_time <= actual_timeout, "Remaining_time {} seconds should be less than watchdog armed timeout {} seconds".format(remaining_timeout, actual_timeout)) remaining_time = watchdog.get_remaining_time(platform_api_conn) time.sleep(TEST_WAIT_TIME_SECONDS) remaining_time_new = watchdog.get_remaining_time(platform_api_conn) - self.expect(remaining_time_new < remaining_time, "remaining_time {} should be decreased from previous remaining_time {}".format(remaining_time_new, remaining_time)) + self.expect(remaining_time_new < remaining_time, "Remaining_time {} seconds should be decreased from previous remaining_time {} seconds".format(remaining_time_new, remaining_time)) self.assert_expectations() + @pytest.mark.dependency(depends=["test_arm_disarm_states"]) def test_periodic_arm(self, duthost, platform_api_conn, conf): ''' arm watchdog several times as watchdog deamon would and verify API behaves correctly ''' @@ -126,10 +130,11 @@ def test_periodic_arm(self, duthost, platform_api_conn, conf): actual_timeout_new = watchdog.arm(platform_api_conn, watchdog_timeout) remaining_time_new = watchdog.get_remaining_time(platform_api_conn) - self.expect(actual_timeout == actual_timeout_new, "{}: new watchdog timeout {} setting should be same as the previous actual watchdog timeout {}".format(test_periodic_arm.__name__, actual_timeout_new, actual_timeout)) - self.expect(remaining_time_new > remaining_time, "{}: new remaining timeout {} should be bigger than the previous remaining timeout {} by {}".format(test_periodic_arm.__name__, remaining_time_new, remaining_time, TEST_WAIT_TIME_SECONDS)) + self.expect(actual_timeout == actual_timeout_new, "{}: new watchdog timeout {} seconds setting should be same as the previous actual watchdog timeout {} seconds".format(test_periodic_arm.__name__, actual_timeout_new, actual_timeout)) + self.expect(remaining_time_new > remaining_time, "{}: new remaining timeout {} seconds should be bigger than the previous remaining timeout {} seconds by {} seconds".format(test_periodic_arm.__name__, remaining_time_new, remaining_time, TEST_WAIT_TIME_SECONDS)) self.assert_expectations() + @pytest.mark.dependency(depends=["test_arm_disarm_states"]) def test_arm_different_timeout_greater(self, duthost, platform_api_conn, conf): ''' arm the watchdog with greater timeout value and verify new timeout was accepted; If platform accepts only single valid timeout value, @greater_timeout should be None. @@ -139,14 +144,15 @@ def test_arm_different_timeout_greater(self, duthost, platform_api_conn, conf): watchdog_timeout_greater = conf['greater_timeout'] if watchdog_timeout_greater is None: pytest.skip('"greater_timeout" parameter is required for this test case') - actual_timeout_second = watchdog.arm(platform_api_conn, watchdog_timeout) + actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) remaining_time = watchdog.get_remaining_time(platform_api_conn) - actual_timeout_second_second = watchdog.arm(platform_api_conn, watchdog_timeout_greater) - self.expect(actual_timeout_second < actual_timeout_second_second, "{}: 1st timeout {} should be smaller than 2nd timeout {}".format(test_arm_different_timeout_greater.__name__, actual_timeout_second, actual_timeout_second_second)) - remaining_time_second = watchdog.get_remaining_time(platform_api_conn) - self.expect(remaining_time_second > remaining_time, "{}: 2nd remaining_timeout {} should be bigger than 1st remaining timeout {}".format(test_arm_different_timeout_greater.__name__, remaining_time_second, remaining_time)) + actual_timeout_greater = watchdog.arm(platform_api_conn, watchdog_timeout_greater) + self.expect(actual_timeout < actual_timeout_greater, "{}: 1st timeout {} seconds should be smaller than 2nd timeout {} seconds".format(test_arm_different_timeout_greater.__name__, actual_timeout, actual_timeout_greater)) + remaining_time_greater = watchdog.get_remaining_time(platform_api_conn) + self.expect(remaining_time_greater > remaining_time, "{}: 2nd remaining_timeout {} seconds should be bigger than 1st remaining timeout {} seconds".format(test_arm_different_timeout_greater.__name__, remaining_time_greater, remaining_time)) self.assert_expectations() + @pytest.mark.dependency(depends=["test_arm_disarm_states"]) def test_arm_different_timeout_smaller(self, duthost, platform_api_conn, conf): ''' arm the watchdog with smaller timeout value and verify new timeout was accepted; If platform accepts only single valid timeout value, @greater_timeout should be None. @@ -160,11 +166,12 @@ def test_arm_different_timeout_smaller(self, duthost, platform_api_conn, conf): remaining_time = watchdog.get_remaining_time(platform_api_conn) actual_timeout_smaller = watchdog.arm(platform_api_conn, watchdog_timeout_smaller) - self.expect(actual_timeout > actual_timeout_smaller, "{}: 1st timeout {} should be bigger than 2nd timeout {}".format(test_arm_different_timeout_smaller.__name__, actual_timeout, actual_timeout_smaller)) + self.expect(actual_timeout > actual_timeout_smaller, "{}: 1st timeout {} seconds should be bigger than 2nd timeout {} seconds".format(test_arm_different_timeout_smaller.__name__, actual_timeout, actual_timeout_smaller)) remaining_time_smaller = watchdog.get_remaining_time(platform_api_conn) - self.expect(remaining_time_smaller < remaining_time, "{}: 2nd remaining_timeout {} should be smaller than 1st remaining timeout {}".format(test_arm_different_timeout_smaller.__name__, remaining_time_smaller, remaining_time)) + self.expect(remaining_time_smaller < remaining_time, "{}: 2nd remaining_timeout {} seconds should be smaller than 1st remaining timeout {} seconds".format(test_arm_different_timeout_smaller.__name__, remaining_time_smaller, remaining_time)) self.assert_expectations() + @pytest.mark.dependency(depends=["test_arm_disarm_states"]) def test_arm_too_big_timeout(self, duthost, platform_api_conn, conf): ''' try to arm the watchdog with timeout that is too big for hardware watchdog; If no such limitation exist, @too_big_timeout should be None for such platform. @@ -174,13 +181,14 @@ def test_arm_too_big_timeout(self, duthost, platform_api_conn, conf): if watchdog_timeout is None: pytest.skip('"too_big_timeout" parameter is required for this test case') actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) - self.expect(actual_timeout == -1, "{}: watchdog time {} shouldn't be set".format(test_arm_too_big_timeout.__name__, watchdog_timeout)) + self.expect(actual_timeout == -1, "{}: watchdog time {} seconds shouldn't be set for big {} seconds".format(test_arm_too_big_timeout.__name__, actual_timeout, watchdog_timeout)) self.assert_expectations() + @pytest.mark.dependency(depends=["test_arm_disarm_states"]) def test_arm_negative_timeout(self, duthost, platform_api_conn): ''' try to arm the watchdog with negative value ''' watchdog_timeout = -1 actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) - self.expect(actual_timeout == -1, "{}: watchdog time {} shouldn't be set".format(test_arm_negative_timeout.__name__, watchdog_timeout)) + self.expect(actual_timeout == -1, "{}: watchdog time {} seconds shouldn't be set for negative {} seconds".format(test_arm_negative_timeout.__name__, actual_timeout, watchdog_timeout)) self.assert_expectations() From a40ccd92f81f1d4a1d91df9749e9bedd8fc955b6 Mon Sep 17 00:00:00 2001 From: sujinmkang Date: Wed, 15 Jul 2020 00:24:35 -0700 Subject: [PATCH 03/11] review comments --- tests/platform_tests/api/test_watchdog.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/tests/platform_tests/api/test_watchdog.py b/tests/platform_tests/api/test_watchdog.py index 41572fee89..bbc6fc7465 100644 --- a/tests/platform_tests/api/test_watchdog.py +++ b/tests/platform_tests/api/test_watchdog.py @@ -53,10 +53,9 @@ def conf(self, request, duthost): if platform in test_config and hwsku in test_config[platform]: config.update(test_config[platform][hwsku]) - self.expect('valid_timeout' in config, "valid_timeout is not defined in config") + pytest_assert('valid_timeout' in config, "valid_timeout is not defined in config") # make sure watchdog won't reboot the system when test sleeps for @TEST_WAIT_TIME_SECONDS - self.expect(config['valid_timeout'] > TEST_WAIT_TIME_SECONDS * 2, "valid_timeout {} seconds is too short".format(config['valid_timeout'])) - self.assert_expectations() + pytest_assert(config['valid_timeout'] > TEST_WAIT_TIME_SECONDS * 2, "valid_timeout {} seconds is too short".format(config['valid_timeout'])) return config @pytest.mark.dependency() @@ -65,7 +64,10 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): disarm watchdog and verify it is in disarmed state ''' watchdog_timeout = conf['valid_timeout'] - actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) + try: + actual_timeout = int(watchdog.arm(platform_api_conn, watchdog_timeout)) + except: + pytest.fail("actual watchdog timeout value is not an integer!") if self.expect(actual_timeout is not None, "Watchdog.arm is not supported"): if self.expect(actual_timeout != -1, "Failed to arm the watchdog"): @@ -75,7 +77,11 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): if self.expect(watchdog_status is not None, "Failed to retrieve watchdog status"): self.expect(watchdog_status is True, "Watchdog is not armed.") - remaining_time = watchdog.get_remaining_time(platform_api_conn) + try: + remaining_time = int(watchdog.get_remaining_time(platform_api_conn)) + except: + pytest.fail("remaining watchdog timeout value is not an integer!") + if self.expect(remaining_time is not None, "Failed to get the remaining time of watchdog"): self.expect(remaining_time <= watchdog_timeout, "Watchdog remaining_time {} seconds is wrong compared to watchdog timeout {} seocnds".format(remaining_time)) @@ -181,7 +187,7 @@ def test_arm_too_big_timeout(self, duthost, platform_api_conn, conf): if watchdog_timeout is None: pytest.skip('"too_big_timeout" parameter is required for this test case') actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) - self.expect(actual_timeout == -1, "{}: watchdog time {} seconds shouldn't be set for big {} seconds".format(test_arm_too_big_timeout.__name__, actual_timeout, watchdog_timeout)) + self.expect(actual_timeout == -1, "{}: Watchdog should be disarmed, but returned timeout of {} seconds".format(test_arm_too_big_timeout.__name__, watchdog_timeout)) self.assert_expectations() @pytest.mark.dependency(depends=["test_arm_disarm_states"]) @@ -190,5 +196,5 @@ def test_arm_negative_timeout(self, duthost, platform_api_conn): watchdog_timeout = -1 actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) - self.expect(actual_timeout == -1, "{}: watchdog time {} seconds shouldn't be set for negative {} seconds".format(test_arm_negative_timeout.__name__, actual_timeout, watchdog_timeout)) + self.expect(actual_timeout == -1, "{}: Watchdog should be disarmed, but returned timeout of {} seconds".format(test_arm_negative_timeout.__name__, watchdog_timeout)) self.assert_expectations() From 7eea20d5be597dab41bd29c6cc5f2afee879f559 Mon Sep 17 00:00:00 2001 From: sujinmkang Date: Wed, 15 Jul 2020 13:56:48 -0700 Subject: [PATCH 04/11] review comments --- tests/platform_tests/api/test_watchdog.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/platform_tests/api/test_watchdog.py b/tests/platform_tests/api/test_watchdog.py index bbc6fc7465..80639a3f7f 100644 --- a/tests/platform_tests/api/test_watchdog.py +++ b/tests/platform_tests/api/test_watchdog.py @@ -64,12 +64,10 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): disarm watchdog and verify it is in disarmed state ''' watchdog_timeout = conf['valid_timeout'] - try: - actual_timeout = int(watchdog.arm(platform_api_conn, watchdog_timeout)) - except: - pytest.fail("actual watchdog timeout value is not an integer!") + actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) if self.expect(actual_timeout is not None, "Watchdog.arm is not supported"): + pytest_assert(isinstance(actual_timeout, int), actual_timeout appears incorrect) if self.expect(actual_timeout != -1, "Failed to arm the watchdog"): self.expect(actual_timeout >= watchdog_timeout, "Actual watchdog {} seconds apears wrong, should be less than {} seconds".format(actual_timeout, watchdog_timeout)) @@ -77,12 +75,10 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): if self.expect(watchdog_status is not None, "Failed to retrieve watchdog status"): self.expect(watchdog_status is True, "Watchdog is not armed.") - try: - remaining_time = int(watchdog.get_remaining_time(platform_api_conn)) - except: - pytest.fail("remaining watchdog timeout value is not an integer!") + remaining_time = watchdog.get_remaining_time(platform_api_conn) if self.expect(remaining_time is not None, "Failed to get the remaining time of watchdog"): + pytest_assert(isinstance(remaining_time, int), remaining_time appears incorrect) self.expect(remaining_time <= watchdog_timeout, "Watchdog remaining_time {} seconds is wrong compared to watchdog timeout {} seocnds".format(remaining_time)) watchdog_status = watchdog.disarm(platform_api_conn) From 22d2d682b5ffdd17828c43056c8b6578b48c78b3 Mon Sep 17 00:00:00 2001 From: sujinmkang Date: Wed, 15 Jul 2020 21:00:44 -0700 Subject: [PATCH 05/11] review comments --- tests/platform_tests/api/test_watchdog.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/platform_tests/api/test_watchdog.py b/tests/platform_tests/api/test_watchdog.py index 80639a3f7f..27a62c61eb 100644 --- a/tests/platform_tests/api/test_watchdog.py +++ b/tests/platform_tests/api/test_watchdog.py @@ -67,7 +67,7 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) if self.expect(actual_timeout is not None, "Watchdog.arm is not supported"): - pytest_assert(isinstance(actual_timeout, int), actual_timeout appears incorrect) + pytest_assert(isinstance(actual_timeout, int), "actual_timeout appears incorrect") if self.expect(actual_timeout != -1, "Failed to arm the watchdog"): self.expect(actual_timeout >= watchdog_timeout, "Actual watchdog {} seconds apears wrong, should be less than {} seconds".format(actual_timeout, watchdog_timeout)) @@ -78,7 +78,7 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): remaining_time = watchdog.get_remaining_time(platform_api_conn) if self.expect(remaining_time is not None, "Failed to get the remaining time of watchdog"): - pytest_assert(isinstance(remaining_time, int), remaining_time appears incorrect) + pytest_assert(isinstance(remaining_time, int), "remaining_time appears incorrect") self.expect(remaining_time <= watchdog_timeout, "Watchdog remaining_time {} seconds is wrong compared to watchdog timeout {} seocnds".format(remaining_time)) watchdog_status = watchdog.disarm(platform_api_conn) From db78e23a29e04b8e1102b49d478eadf74715a2bc Mon Sep 17 00:00:00 2001 From: sujinmkang Date: Thu, 16 Jul 2020 12:12:36 -0700 Subject: [PATCH 06/11] review comment --- tests/platform_tests/api/test_watchdog.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/platform_tests/api/test_watchdog.py b/tests/platform_tests/api/test_watchdog.py index 27a62c61eb..56fceea92b 100644 --- a/tests/platform_tests/api/test_watchdog.py +++ b/tests/platform_tests/api/test_watchdog.py @@ -69,7 +69,7 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): if self.expect(actual_timeout is not None, "Watchdog.arm is not supported"): pytest_assert(isinstance(actual_timeout, int), "actual_timeout appears incorrect") if self.expect(actual_timeout != -1, "Failed to arm the watchdog"): - self.expect(actual_timeout >= watchdog_timeout, "Actual watchdog {} seconds apears wrong, should be less than {} seconds".format(actual_timeout, watchdog_timeout)) + self.expect(actual_timeout >= watchdog_timeout, "Actual watchdog {} seconds apear wrong, should be less than {} seconds".format(actual_timeout, watchdog_timeout)) watchdog_status = watchdog.is_armed(platform_api_conn) if self.expect(watchdog_status is not None, "Failed to retrieve watchdog status"): @@ -79,7 +79,7 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): if self.expect(remaining_time is not None, "Failed to get the remaining time of watchdog"): pytest_assert(isinstance(remaining_time, int), "remaining_time appears incorrect") - self.expect(remaining_time <= watchdog_timeout, "Watchdog remaining_time {} seconds is wrong compared to watchdog timeout {} seocnds".format(remaining_time)) + self.expect(remaining_time <= watchdog_timeout, "Watchdog remaining_time {} seconds appear wrong compared to watchdog timeout {} seocnds".format(remaining_time)) watchdog_status = watchdog.disarm(platform_api_conn) if self.expect(watchdog_status is not None, "Watchdog.disarm is not supported"): From 633cf844e1fe1e2b023c0214b8b8c6136a953097 Mon Sep 17 00:00:00 2001 From: sujinmkang Date: Thu, 16 Jul 2020 12:29:36 -0700 Subject: [PATCH 07/11] review comment --- tests/platform_tests/api/test_watchdog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/platform_tests/api/test_watchdog.py b/tests/platform_tests/api/test_watchdog.py index 56fceea92b..26ada2fbfc 100644 --- a/tests/platform_tests/api/test_watchdog.py +++ b/tests/platform_tests/api/test_watchdog.py @@ -69,7 +69,7 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): if self.expect(actual_timeout is not None, "Watchdog.arm is not supported"): pytest_assert(isinstance(actual_timeout, int), "actual_timeout appears incorrect") if self.expect(actual_timeout != -1, "Failed to arm the watchdog"): - self.expect(actual_timeout >= watchdog_timeout, "Actual watchdog {} seconds apear wrong, should be less than {} seconds".format(actual_timeout, watchdog_timeout)) + self.expect(actual_timeout >= watchdog_timeout, "Actual watchdog timeout {} seconds apear wrong, should be less than {} seconds".format(actual_timeout, watchdog_timeout)) watchdog_status = watchdog.is_armed(platform_api_conn) if self.expect(watchdog_status is not None, "Failed to retrieve watchdog status"): From ae4d370b13a68ae54b4fb7dc4b50d9ec1117671c Mon Sep 17 00:00:00 2001 From: sujinmkang Date: Thu, 16 Jul 2020 15:05:14 -0700 Subject: [PATCH 08/11] typo --- tests/platform_tests/api/test_watchdog.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/platform_tests/api/test_watchdog.py b/tests/platform_tests/api/test_watchdog.py index 26ada2fbfc..3367c5b811 100644 --- a/tests/platform_tests/api/test_watchdog.py +++ b/tests/platform_tests/api/test_watchdog.py @@ -69,7 +69,7 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): if self.expect(actual_timeout is not None, "Watchdog.arm is not supported"): pytest_assert(isinstance(actual_timeout, int), "actual_timeout appears incorrect") if self.expect(actual_timeout != -1, "Failed to arm the watchdog"): - self.expect(actual_timeout >= watchdog_timeout, "Actual watchdog timeout {} seconds apear wrong, should be less than {} seconds".format(actual_timeout, watchdog_timeout)) + self.expect(actual_timeout >= watchdog_timeout, "Actual watchdog timeout {} seconds appears wrong, should be less than {} seconds".format(actual_timeout, watchdog_timeout)) watchdog_status = watchdog.is_armed(platform_api_conn) if self.expect(watchdog_status is not None, "Failed to retrieve watchdog status"): @@ -79,7 +79,7 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): if self.expect(remaining_time is not None, "Failed to get the remaining time of watchdog"): pytest_assert(isinstance(remaining_time, int), "remaining_time appears incorrect") - self.expect(remaining_time <= watchdog_timeout, "Watchdog remaining_time {} seconds appear wrong compared to watchdog timeout {} seocnds".format(remaining_time)) + self.expect(remaining_time <= watchdog_timeout, "Watchdog remaining_time {} seconds appears wrong compared to watchdog timeout {} seocnds".format(remaining_time)) watchdog_status = watchdog.disarm(platform_api_conn) if self.expect(watchdog_status is not None, "Watchdog.disarm is not supported"): From b3b05fa50ffa53560a3bd53971523bed4c4414d1 Mon Sep 17 00:00:00 2001 From: sujinmkang Date: Thu, 16 Jul 2020 16:36:29 -0700 Subject: [PATCH 09/11] review comment --- tests/platform_tests/api/test_watchdog.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/platform_tests/api/test_watchdog.py b/tests/platform_tests/api/test_watchdog.py index 3367c5b811..cd00271156 100644 --- a/tests/platform_tests/api/test_watchdog.py +++ b/tests/platform_tests/api/test_watchdog.py @@ -67,9 +67,9 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) if self.expect(actual_timeout is not None, "Watchdog.arm is not supported"): - pytest_assert(isinstance(actual_timeout, int), "actual_timeout appears incorrect") - if self.expect(actual_timeout != -1, "Failed to arm the watchdog"): - self.expect(actual_timeout >= watchdog_timeout, "Actual watchdog timeout {} seconds appears wrong, should be less than {} seconds".format(actual_timeout, watchdog_timeout)) + if self.expect(isinstance(actual_timeout, int), "actual_timeout appears incorrect") + if self.expect(actual_timeout != -1, "Failed to arm the watchdog"): + self.expect(actual_timeout >= watchdog_timeout, "Actual watchdog timeout {} seconds appears wrong, should be less than {} seconds".format(actual_timeout, watchdog_timeout)) watchdog_status = watchdog.is_armed(platform_api_conn) if self.expect(watchdog_status is not None, "Failed to retrieve watchdog status"): @@ -78,8 +78,8 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): remaining_time = watchdog.get_remaining_time(platform_api_conn) if self.expect(remaining_time is not None, "Failed to get the remaining time of watchdog"): - pytest_assert(isinstance(remaining_time, int), "remaining_time appears incorrect") - self.expect(remaining_time <= watchdog_timeout, "Watchdog remaining_time {} seconds appears wrong compared to watchdog timeout {} seocnds".format(remaining_time)) + if self.expect(isinstance(remaining_time, int), "remaining_time appears incorrect") + self.expect(remaining_time <= watchdog_timeout, "Watchdog remaining_time {} seconds appears wrong compared to watchdog timeout {} seocnds".format(remaining_time)) watchdog_status = watchdog.disarm(platform_api_conn) if self.expect(watchdog_status is not None, "Watchdog.disarm is not supported"): From cbb80f1d2849c1232cfce76996aede28e3dcdffd Mon Sep 17 00:00:00 2001 From: sujinmkang Date: Fri, 17 Jul 2020 10:45:34 -0700 Subject: [PATCH 10/11] lgtm --- tests/platform_tests/api/test_watchdog.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/platform_tests/api/test_watchdog.py b/tests/platform_tests/api/test_watchdog.py index cd00271156..ee11ce8351 100644 --- a/tests/platform_tests/api/test_watchdog.py +++ b/tests/platform_tests/api/test_watchdog.py @@ -67,7 +67,7 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) if self.expect(actual_timeout is not None, "Watchdog.arm is not supported"): - if self.expect(isinstance(actual_timeout, int), "actual_timeout appears incorrect") + if self.expect(isinstance(actual_timeout, int), "actual_timeout appears incorrect"): if self.expect(actual_timeout != -1, "Failed to arm the watchdog"): self.expect(actual_timeout >= watchdog_timeout, "Actual watchdog timeout {} seconds appears wrong, should be less than {} seconds".format(actual_timeout, watchdog_timeout)) @@ -78,7 +78,7 @@ def test_arm_disarm_states(self, duthost, localhost, platform_api_conn, conf): remaining_time = watchdog.get_remaining_time(platform_api_conn) if self.expect(remaining_time is not None, "Failed to get the remaining time of watchdog"): - if self.expect(isinstance(remaining_time, int), "remaining_time appears incorrect") + if self.expect(isinstance(remaining_time, int), "remaining_time appears incorrect"): self.expect(remaining_time <= watchdog_timeout, "Watchdog remaining_time {} seconds appears wrong compared to watchdog timeout {} seocnds".format(remaining_time)) watchdog_status = watchdog.disarm(platform_api_conn) From 108cb0cd4ee1d73570630b6ed07cf53d471e314b Mon Sep 17 00:00:00 2001 From: sujinmkang Date: Fri, 17 Jul 2020 17:00:41 -0700 Subject: [PATCH 11/11] review comments --- tests/platform_tests/api/test_watchdog.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/platform_tests/api/test_watchdog.py b/tests/platform_tests/api/test_watchdog.py index ee11ce8351..3645db1eaf 100644 --- a/tests/platform_tests/api/test_watchdog.py +++ b/tests/platform_tests/api/test_watchdog.py @@ -133,7 +133,7 @@ def test_periodic_arm(self, duthost, platform_api_conn, conf): remaining_time_new = watchdog.get_remaining_time(platform_api_conn) self.expect(actual_timeout == actual_timeout_new, "{}: new watchdog timeout {} seconds setting should be same as the previous actual watchdog timeout {} seconds".format(test_periodic_arm.__name__, actual_timeout_new, actual_timeout)) - self.expect(remaining_time_new > remaining_time, "{}: new remaining timeout {} seconds should be bigger than the previous remaining timeout {} seconds by {} seconds".format(test_periodic_arm.__name__, remaining_time_new, remaining_time, TEST_WAIT_TIME_SECONDS)) + self.expect(remaining_time_new > remaining_time, "{}: new remaining timeout {} seconds should be greater than the previous remaining timeout {} seconds by {} seconds".format(test_periodic_arm.__name__, remaining_time_new, remaining_time, TEST_WAIT_TIME_SECONDS)) self.assert_expectations() @pytest.mark.dependency(depends=["test_arm_disarm_states"]) @@ -149,9 +149,9 @@ def test_arm_different_timeout_greater(self, duthost, platform_api_conn, conf): actual_timeout = watchdog.arm(platform_api_conn, watchdog_timeout) remaining_time = watchdog.get_remaining_time(platform_api_conn) actual_timeout_greater = watchdog.arm(platform_api_conn, watchdog_timeout_greater) - self.expect(actual_timeout < actual_timeout_greater, "{}: 1st timeout {} seconds should be smaller than 2nd timeout {} seconds".format(test_arm_different_timeout_greater.__name__, actual_timeout, actual_timeout_greater)) + self.expect(actual_timeout < actual_timeout_greater, "{}: 1st timeout {} seconds should be less than 2nd timeout {} seconds".format(test_arm_different_timeout_greater.__name__, actual_timeout, actual_timeout_greater)) remaining_time_greater = watchdog.get_remaining_time(platform_api_conn) - self.expect(remaining_time_greater > remaining_time, "{}: 2nd remaining_timeout {} seconds should be bigger than 1st remaining timeout {} seconds".format(test_arm_different_timeout_greater.__name__, remaining_time_greater, remaining_time)) + self.expect(remaining_time_greater > remaining_time, "{}: 2nd remaining_timeout {} seconds should be greater than 1st remaining timeout {} seconds".format(test_arm_different_timeout_greater.__name__, remaining_time_greater, remaining_time)) self.assert_expectations() @pytest.mark.dependency(depends=["test_arm_disarm_states"]) @@ -168,9 +168,9 @@ def test_arm_different_timeout_smaller(self, duthost, platform_api_conn, conf): remaining_time = watchdog.get_remaining_time(platform_api_conn) actual_timeout_smaller = watchdog.arm(platform_api_conn, watchdog_timeout_smaller) - self.expect(actual_timeout > actual_timeout_smaller, "{}: 1st timeout {} seconds should be bigger than 2nd timeout {} seconds".format(test_arm_different_timeout_smaller.__name__, actual_timeout, actual_timeout_smaller)) + self.expect(actual_timeout > actual_timeout_smaller, "{}: 1st timeout {} seconds should be greater than 2nd timeout {} seconds".format(test_arm_different_timeout_smaller.__name__, actual_timeout, actual_timeout_smaller)) remaining_time_smaller = watchdog.get_remaining_time(platform_api_conn) - self.expect(remaining_time_smaller < remaining_time, "{}: 2nd remaining_timeout {} seconds should be smaller than 1st remaining timeout {} seconds".format(test_arm_different_timeout_smaller.__name__, remaining_time_smaller, remaining_time)) + self.expect(remaining_time_smaller < remaining_time, "{}: 2nd remaining_timeout {} seconds should be less than 1st remaining timeout {} seconds".format(test_arm_different_timeout_smaller.__name__, remaining_time_smaller, remaining_time)) self.assert_expectations() @pytest.mark.dependency(depends=["test_arm_disarm_states"])