Skip to content

Commit 1f4bd40

Browse files
yutongzhang-microsoftyutongzhang-microsoft
authored
Refactor fixture start_platform_api_service for cleaner organization (sonic-net#17428)
What is the motivation for this PR? In PR sonic-net#16860, we moved the start_platform_api_service fixture to tests/conftest.py, but this approach is neither elegant nor clean. Since it is used by the platform test fixture platform_api_conn, we have relocated it to the same file as platform_api_conn for better organization. Additionally, because pytest does not automatically detect fixtures outside of conftest.py, we now need to explicitly import both start_platform_api_service and platform_api_conn. How did you do it? Since the fixture start_platform_api_service is used by the platform test fixture platform_api_conn, we have relocated it to the same file as platform_api_conn for better organization. Additionally, because pytest does not automatically detect fixtures outside of conftest.py, we now need to explicitly import both start_platform_api_service and platform_api_conn. How did you verify/test it? Perviously, to avoid the error fixture 'start_platform_api_service' not found, we moved this fixture to tests/conftest.py. This time, I have tested locally, as we explicitly import both start_platform_api_service and platform_api_conn, there is no such error.
1 parent 798677d commit 1f4bd40

16 files changed

+65
-66
lines changed

‎tests/common/platform/device_utils.py

+51
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@
4747
FMT_SHORT = "%b %d %H:%M:%S"
4848
FMT_ALT = "%Y-%m-%dT%H:%M:%S.%f%z"
4949

50+
SERVER_FILE = 'platform_api_server.py'
51+
SERVER_PORT = 8000
52+
IPTABLES_PREPEND_RULE_CMD = 'iptables -I INPUT 1 -p tcp -m tcp --dport {} -j ACCEPT'.format(SERVER_PORT)
53+
5054
test_report = dict()
5155

5256

@@ -982,6 +986,53 @@ def advanceboot_neighbor_restore(duthosts, enum_rand_one_per_hwsku_frontend_host
982986
neighbor_vm_restore(duthost, nbrhosts, tbinfo)
983987

984988

989+
@pytest.fixture(scope='function')
990+
def start_platform_api_service(duthosts, enum_rand_one_per_hwsku_hostname, localhost, request):
991+
duthost = duthosts[enum_rand_one_per_hwsku_hostname]
992+
dut_ip = duthost.mgmt_ip
993+
994+
res = localhost.wait_for(host=dut_ip,
995+
port=SERVER_PORT,
996+
state='started',
997+
delay=1,
998+
timeout=10,
999+
module_ignore_errors=True)
1000+
if res['failed'] is True:
1001+
1002+
res = duthost.command('docker exec -i pmon python3 -c "import sonic_platform"', module_ignore_errors=True)
1003+
py3_platform_api_available = not res['failed']
1004+
1005+
supervisor_conf = [
1006+
'[program:platform_api_server]',
1007+
'command=/usr/bin/python{} /opt/platform_api_server.py --port {}'.format('3' if py3_platform_api_available
1008+
else '2', SERVER_PORT),
1009+
'autostart=True',
1010+
'autorestart=True',
1011+
'stdout_logfile=syslog',
1012+
'stderr_logfile=syslog',
1013+
]
1014+
dest_path = os.path.join(os.sep, 'tmp', 'platform_api_server.conf')
1015+
pmon_path = os.path.join(os.sep, 'etc', 'supervisor', 'conf.d', 'platform_api_server.conf')
1016+
duthost.copy(content='\n'.join(supervisor_conf), dest=dest_path)
1017+
duthost.command('docker cp {} pmon:{}'.format(dest_path, pmon_path))
1018+
1019+
src_path = os.path.join('common', 'helpers', 'platform_api', 'scripts', SERVER_FILE)
1020+
dest_path = os.path.join(os.sep, 'tmp', SERVER_FILE)
1021+
pmon_path = os.path.join(os.sep, 'opt', SERVER_FILE)
1022+
duthost.copy(src=src_path, dest=dest_path)
1023+
duthost.command('docker cp {} pmon:{}'.format(dest_path, pmon_path))
1024+
1025+
# Prepend an iptables rule to allow incoming traffic to the HTTP server
1026+
duthost.command(IPTABLES_PREPEND_RULE_CMD)
1027+
1028+
# Reload the supervisor config and Start the HTTP server
1029+
duthost.command('docker exec -i pmon supervisorctl reread')
1030+
duthost.command('docker exec -i pmon supervisorctl update')
1031+
1032+
res = localhost.wait_for(host=dut_ip, port=SERVER_PORT, state='started', delay=1, timeout=10)
1033+
assert res['failed'] is False
1034+
1035+
9851036
@pytest.fixture(scope='function')
9861037
def platform_api_conn(duthosts, enum_rand_one_per_hwsku_hostname, start_platform_api_service):
9871038
duthost = duthosts[enum_rand_one_per_hwsku_hostname]

‎tests/conftest.py

-51
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,6 @@
7676
DUTHOSTS_FIXTURE_FAILED_RC = 15
7777
CUSTOM_MSG_PREFIX = "sonic_custom_msg"
7878

79-
SERVER_FILE = 'platform_api_server.py'
80-
SERVER_PORT = 8000
81-
IPTABLES_PREPEND_RULE_CMD = 'iptables -I INPUT 1 -p tcp -m tcp --dport {} -j ACCEPT'.format(SERVER_PORT)
82-
8379
pytest_plugins = ('tests.common.plugins.ptfadapter',
8480
'tests.common.plugins.ansible_fixtures',
8581
'tests.common.plugins.dut_monitor',
@@ -2865,53 +2861,6 @@ def gnxi_path(ptfhost):
28652861
return gnxipath
28662862

28672863

2868-
@pytest.fixture(scope='function')
2869-
def start_platform_api_service(duthosts, enum_rand_one_per_hwsku_hostname, localhost, request):
2870-
duthost = duthosts[enum_rand_one_per_hwsku_hostname]
2871-
dut_ip = duthost.mgmt_ip
2872-
2873-
res = localhost.wait_for(host=dut_ip,
2874-
port=SERVER_PORT,
2875-
state='started',
2876-
delay=1,
2877-
timeout=10,
2878-
module_ignore_errors=True)
2879-
if res['failed'] is True:
2880-
2881-
res = duthost.command('docker exec -i pmon python3 -c "import sonic_platform"', module_ignore_errors=True)
2882-
py3_platform_api_available = not res['failed']
2883-
2884-
supervisor_conf = [
2885-
'[program:platform_api_server]',
2886-
'command=/usr/bin/python{} /opt/platform_api_server.py --port {}'.format('3' if py3_platform_api_available
2887-
else '2', SERVER_PORT),
2888-
'autostart=True',
2889-
'autorestart=True',
2890-
'stdout_logfile=syslog',
2891-
'stderr_logfile=syslog',
2892-
]
2893-
dest_path = os.path.join(os.sep, 'tmp', 'platform_api_server.conf')
2894-
pmon_path = os.path.join(os.sep, 'etc', 'supervisor', 'conf.d', 'platform_api_server.conf')
2895-
duthost.copy(content='\n'.join(supervisor_conf), dest=dest_path)
2896-
duthost.command('docker cp {} pmon:{}'.format(dest_path, pmon_path))
2897-
2898-
src_path = os.path.join('common', 'helpers', 'platform_api', 'scripts', SERVER_FILE)
2899-
dest_path = os.path.join(os.sep, 'tmp', SERVER_FILE)
2900-
pmon_path = os.path.join(os.sep, 'opt', SERVER_FILE)
2901-
duthost.copy(src=src_path, dest=dest_path)
2902-
duthost.command('docker cp {} pmon:{}'.format(dest_path, pmon_path))
2903-
2904-
# Prepend an iptables rule to allow incoming traffic to the HTTP server
2905-
duthost.command(IPTABLES_PREPEND_RULE_CMD)
2906-
2907-
# Reload the supervisor config and Start the HTTP server
2908-
duthost.command('docker exec -i pmon supervisorctl reread')
2909-
duthost.command('docker exec -i pmon supervisorctl update')
2910-
2911-
res = localhost.wait_for(host=dut_ip, port=SERVER_PORT, state='started', delay=1, timeout=10)
2912-
assert res['failed'] is False
2913-
2914-
29152864
def pytest_collection_modifyitems(config, items):
29162865
# Skip all stress_tests if --run-stress-test is not set
29172866
if not config.getoption("--run-stress-tests"):

‎tests/platform_tests/api/test_chassis.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
from tests.common.utilities import get_host_visible_vars
1212
from tests.common.utilities import skip_release
1313
from tests.common.platform.interface_utils import get_physical_port_indices
14-
from tests.common.platform.device_utils import platform_api_conn # noqa F401
14+
from tests.common.platform.device_utils import platform_api_conn, start_platform_api_service # noqa F401
1515
from tests.platform_tests.cli.util import get_skip_mod_list
1616
from .platform_api_test_base import PlatformApiTestBase
1717

‎tests/platform_tests/api/test_chassis_fans.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
from tests.common.helpers.platform_api import chassis, fan
77
from .platform_api_test_base import PlatformApiTestBase
8-
from tests.common.platform.device_utils import platform_api_conn # noqa F401
8+
from tests.common.platform.device_utils import platform_api_conn, start_platform_api_service # noqa F401
99
from tests.common.helpers.thermal_control_test_helper import start_thermal_control_daemon, stop_thermal_control_daemon
1010

1111
###################################################

‎tests/platform_tests/api/test_component.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from tests.common.helpers.platform_api import chassis, component
66
from .platform_api_test_base import PlatformApiTestBase
77
from tests.common.utilities import skip_release_for_platform
8-
from tests.common.platform.device_utils import platform_api_conn # noqa F401
8+
from tests.common.platform.device_utils import platform_api_conn, start_platform_api_service # noqa F401
99

1010
###################################################
1111
# TODO: Remove this after we transition to Python 3

‎tests/platform_tests/api/test_fan_drawer.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import pytest
44

55
from tests.common.helpers.platform_api import chassis, fan_drawer
6-
from tests.common.platform.device_utils import platform_api_conn # noqa F401
6+
from tests.common.platform.device_utils import platform_api_conn, start_platform_api_service # noqa F401
77

88
from .platform_api_test_base import PlatformApiTestBase
99

‎tests/platform_tests/api/test_fan_drawer_fans.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
from tests.common.helpers.platform_api import chassis, fan_drawer, fan_drawer_fan
88
from tests.common.helpers.thermal_control_test_helper import start_thermal_control_daemon, stop_thermal_control_daemon
9-
from tests.common.platform.device_utils import platform_api_conn # noqa F401
9+
from tests.common.platform.device_utils import platform_api_conn, start_platform_api_service # noqa F401
1010
from .platform_api_test_base import PlatformApiTestBase
1111

1212
###################################################

‎tests/platform_tests/api/test_module.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from .platform_api_test_base import PlatformApiTestBase
99
from tests.common.helpers.assertions import pytest_assert
1010
from tests.common.helpers.dut_utils import ignore_t2_syslog_msgs
11-
from tests.common.platform.device_utils import platform_api_conn # noqa F401
11+
from tests.common.platform.device_utils import platform_api_conn, start_platform_api_service # noqa F401
1212

1313
###################################################
1414
# TODO: Remove this after we transition to Python 3

‎tests/platform_tests/api/test_psu.py

+1-2
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@
77
from tests.platform_tests.cli.util import get_skip_mod_list
88
from .platform_api_test_base import PlatformApiTestBase
99
from tests.common.utilities import skip_release_for_platform, wait_until
10-
from tests.common.platform.device_utils import platform_api_conn # noqa F401
11-
10+
from tests.common.platform.device_utils import platform_api_conn, start_platform_api_service # noqa F401
1211

1312
###################################################
1413
# TODO: Remove this after we transition to Python 3

‎tests/platform_tests/api/test_psu_fans.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import pytest
66

77
from tests.common.helpers.platform_api import chassis, psu, psu_fan
8-
from tests.common.platform.device_utils import platform_api_conn # noqa F401
8+
from tests.common.platform.device_utils import platform_api_conn, start_platform_api_service # noqa F401
99

1010
from .platform_api_test_base import PlatformApiTestBase
1111

‎tests/platform_tests/api/test_sfp.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
from tests.common.utilities import wait_until
1515
from tests.common.fixtures.conn_graph_facts import conn_graph_facts # noqa F401
1616
from tests.common.fixtures.duthost_utils import shutdown_ebgp # noqa F401
17-
from tests.common.platform.device_utils import platform_api_conn # noqa F401
17+
from tests.common.platform.device_utils import platform_api_conn, start_platform_api_service # noqa F401
1818
from tests.common.platform.transceiver_utils import is_sw_control_enabled,\
1919
get_port_expected_error_state_for_mellanox_device_on_sw_control_enabled
2020
from tests.common.mellanox_data import is_mellanox_device

‎tests/platform_tests/api/test_thermal.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
from tests.common.helpers.platform_api import chassis, thermal
55
from tests.common.utilities import skip_release_for_platform
6-
from tests.common.platform.device_utils import platform_api_conn # noqa F401
6+
from tests.common.platform.device_utils import platform_api_conn, start_platform_api_service # noqa F401
77

88
from .platform_api_test_base import PlatformApiTestBase
99

‎tests/platform_tests/api/test_watchdog.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import pytest
77
from tests.common.helpers.platform_api import watchdog
88
from tests.common.helpers.assertions import pytest_assert
9-
from tests.common.platform.device_utils import platform_api_conn # noqa F401
9+
from tests.common.platform.device_utils import platform_api_conn, start_platform_api_service # noqa F401
1010
from .platform_api_test_base import PlatformApiTestBase
1111

1212
from collections import OrderedDict

‎tests/smartswitch/common/device_utils_dpu.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import logging
55
import pytest
66
import re
7-
from tests.common.platform.device_utils import platform_api_conn # noqa: F401,F403
7+
from tests.common.platform.device_utils import platform_api_conn, start_platform_api_service # noqa: F401,F403
88
from tests.common.helpers.platform_api import chassis, module
99
from tests.common.utilities import wait_until
1010
from tests.common.helpers.assertions import pytest_assert

‎tests/smartswitch/platform_tests/test_platform_dpu.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
parse_dpu_memory_usage, parse_system_health_summary,\
1414
pre_test_check, post_test_dpu_check,\
1515
check_dpu_health_status, num_dpu_modules # noqa: F401
16-
from tests.common.platform.device_utils import platform_api_conn # noqa: F401,F403
16+
from tests.common.platform.device_utils import platform_api_conn, start_platform_api_service # noqa: F401,F403
1717

1818
pytestmark = [
1919
pytest.mark.topology('smartswitch')

‎tests/smartswitch/platform_tests/test_reload_dpu.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
check_dpu_ping_status, check_dpu_link_and_status, check_dpu_module_status,\
1212
pre_test_check, post_test_switch_check, post_test_dpu_check,\
1313
check_dpu_reboot_cause, num_dpu_modules # noqa: F401
14-
from tests.common.platform.device_utils import platform_api_conn # noqa: F401,F403
14+
from tests.common.platform.device_utils import platform_api_conn, start_platform_api_service # noqa: F401,F403
1515

1616
pytestmark = [
1717
pytest.mark.topology('smartswitch')

0 commit comments

Comments
 (0)