Skip to content

Commit

Permalink
Increased pcied unit test coverage to > 80% (sonic-net#243)
Browse files Browse the repository at this point in the history
  • Loading branch information
assrinivasan authored Mar 4, 2022
1 parent 7d7c85e commit c092300
Show file tree
Hide file tree
Showing 9 changed files with 263 additions and 18 deletions.
3 changes: 1 addition & 2 deletions sonic-pcied/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,10 @@
'enum34; python_version < "3.4"',
'sonic-py-common',
],
tests_requires=[
tests_require=[
'mock>=2.0.0; python_version < "3.3"',
'pytest',
'pytest-cov',
'sonic-platform-common'
],
classifiers=[
'Development Status :: 4 - Beta',
Expand Down
6 changes: 6 additions & 0 deletions sonic-pcied/tests/mocked_libs/sonic_platform/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"""
Mock implementation of sonic_platform package for unit testing
"""

from . import pcie

13 changes: 13 additions & 0 deletions sonic-pcied/tests/mocked_libs/sonic_platform/pcie.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
"""
Mock implementation of sonic_platform package for unit testing
"""

from sonic_platform_base.pcie_base import PcieBase


class Pcie(PcieBase):
def __init__(self):
self.platform_pcieutil = "/tmp/Pcie"

def __str__(self):
return self.platform_pcieutil
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#
# pcie_base.py
#
# Abstract base class for implementing platform-specific
# PCIE functionality for SONiC
#

try:
import abc
except ImportError as e:
raise ImportError(str(e) + " - required module not found")


class PcieBase(object):
def __init__(self, path):
"""
Constructor
Args:
pcieutil file and config file path
"""

@abc.abstractmethod
def get_pcie_device(self):
"""
get current device pcie info
Returns:
A list including pcie device info
"""
return []

@abc.abstractmethod
def get_pcie_check(self):
"""
Check Pcie device with config file
Returns:
A list including pcie device and test result info
"""
return []

@abc.abstractmethod
def get_pcie_aer_stats(self, domain, bus, dev, fn):
"""
Returns a nested dictionary containing the AER stats belonging to a
PCIe device
Args:
domain, bus, dev, fn: Domain, bus, device, function of the PCIe
device respectively
Returns:
A nested dictionary where key is severity 'correctable', 'fatal' or
'non_fatal', value is a dictionary of key, value pairs in the format:
{'AER Error type': Error count}
Ex. {'correctable': {'BadDLLP': 0, 'BadTLP': 0},
'fatal': {'RxOF': 0, 'MalfTLP': 0},
'non_fatal': {'RxOF': 0, 'MalfTLP': 0}}
For PCIe devices that do not support AER, the value for each
severity key is an empty dictionary.
"""
return {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
# pcie_common.py
# Common PCIE check interfaces for SONIC
#

import os
import yaml
import subprocess
import re
import sys
from copy import deepcopy
try:
from .pcie_base import PcieBase
except ImportError as e:
raise ImportError(str(e) + "- required module not found")


class PcieUtil(PcieBase):
"""Platform-specific PCIEutil class"""
# got the config file path
def __init__(self, path):
self.config_path = path
self._conf_rev = None

# load the config file
def load_config_file(self):
conf_rev = "_{}".format(self._conf_rev) if self._conf_rev else ""
config_file = "{}/pcie{}.yaml".format(self.config_path, conf_rev)
try:
with open(config_file) as conf_file:
self.confInfo = yaml.safe_load(conf_file)
except IOError as e:
print("Error: {}".format(str(e)))
print("Not found config file, please add a config file manually, or generate it by running [pcieutil pcie_generate]")
sys.exit()

# load current PCIe device
def get_pcie_device(self):
pciDict = {}
pciList = []
p1 = "^(\w+):(\w+)\.(\w)\s(.*)\s*\(*.*\)*"
p2 = "^.*:.*:.*:(\w+)\s*\(*.*\)*"
command1 = "sudo lspci"
command2 = "sudo lspci -n"
# run command 1
proc1 = subprocess.Popen(command1, shell=True, universal_newlines=True, stdout=subprocess.PIPE)
output1 = proc1.stdout.readlines()
(out, err) = proc1.communicate()
# run command 2
proc2 = subprocess.Popen(command2, shell=True, universal_newlines=True, stdout=subprocess.PIPE)
output2 = proc2.stdout.readlines()
(out, err) = proc2.communicate()

if proc1.returncode > 0:
for line1 in output1:
print(line1.strip())
return
elif proc2.returncode > 0:
for line2 in output2:
print(line2.strip())
return
else:
for (line1, line2) in zip(output1, output2):
pciDict.clear()
match1 = re.search(p1, line1.strip())
match2 = re.search(p2, line2.strip())
if match1 and match2:
Bus = match1.group(1)
Dev = match1.group(2)
Fn = match1.group(3)
Name = match1.group(4)
Id = match2.group(1)
pciDict["name"] = Name
pciDict["bus"] = Bus
pciDict["dev"] = Dev
pciDict["fn"] = Fn
pciDict["id"] = Id
pciList.append(pciDict)
pciDict = deepcopy(pciDict)
else:
print("CAN NOT MATCH PCIe DEVICE")
return pciList

# check the sysfs tree for each PCIe device
def check_pcie_sysfs(self, domain=0, bus=0, device=0, func=0):
dev_path = os.path.join('/sys/bus/pci/devices', '%04x:%02x:%02x.%d' % (domain, bus, device, func))
if os.path.exists(dev_path):
return True
return False

# check the current PCIe device with config file and return the result
def get_pcie_check(self):
self.load_config_file()
for item_conf in self.confInfo:
bus_conf = item_conf["bus"]
dev_conf = item_conf["dev"]
fn_conf = item_conf["fn"]
if self.check_pcie_sysfs(bus=int(bus_conf, base=16), device=int(dev_conf, base=16), func=int(fn_conf, base=16)):
item_conf["result"] = "Passed"
else:
item_conf["result"] = "Failed"
return self.confInfo

# return AER stats of PCIe device
def get_pcie_aer_stats(self, domain=0, bus=0, dev=0, func=0):
aer_stats = {'correctable': {}, 'fatal': {}, 'non_fatal': {}}
dev_path = os.path.join('/sys/bus/pci/devices', '%04x:%02x:%02x.%d' % (domain, bus, dev, func))

# construct AER sysfs filepath
correctable_path = os.path.join(dev_path, "aer_dev_correctable")
fatal_path = os.path.join(dev_path, "aer_dev_fatal")
non_fatal_path = os.path.join(dev_path, "aer_dev_nonfatal")

# update AER-correctable fields
if os.path.isfile(correctable_path):
with open(correctable_path, 'r') as fh:
lines = fh.readlines()
for line in lines:
correctable_field, value = line.split()
aer_stats['correctable'][correctable_field] = value

# update AER-Fatal fields
if os.path.isfile(fatal_path):
with open(fatal_path, 'r') as fh:
lines = fh.readlines()
for line in lines:
fatal_field, value = line.split()
aer_stats['fatal'][fatal_field] = value

# update AER-Non Fatal fields
if os.path.isfile(non_fatal_path):
with open(non_fatal_path, 'r') as fh:
lines = fh.readlines()
for line in lines:
non_fatal_field, value = line.split()
aer_stats['non_fatal'][non_fatal_field] = value

return aer_stats

# generate the config file with current pci device
def dump_conf_yaml(self):
curInfo = self.get_pcie_device()
conf_rev = "_{}".format(self._conf_rev) if self._conf_rev else ""
config_file = "{}/pcie{}.yaml".format(self.config_path, conf_rev)
with open(config_file, "w") as conf_file:
yaml.dump(curInfo, conf_file, default_flow_style=False)
return
15 changes: 7 additions & 8 deletions sonic-pcied/tests/test_DaemonPcied.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,10 @@
{'correctable': {}, 'fatal': {}, 'non_fatal': {}}
"""

pcie_aer_stats_no_err = \
"""
{'correctable': {'field1': '0', 'field2': '0'},
pcie_aer_stats_no_err = {'correctable': {'field1': '0', 'field2': '0'},
'fatal': {'field3': '0', 'field4': '0'},
'non_fatal': {'field5': '0', 'field6': '0'}}
"""


pcie_aer_stats_err = \
"""
Expand Down Expand Up @@ -207,9 +205,9 @@ def test_update_aer_to_statedb(self):
daemon_pcied.log_debug = mock.MagicMock()
daemon_pcied.device_table = mock.MagicMock()
daemon_pcied.device_name = mock.MagicMock()
daemon_pcied.aer_stats = mock.MagicMock()

daemon_pcied.aer_stats = pcie_aer_stats_no_err

"""
mocked_expected_fvp = pcied.swsscommon.FieldValuePairs(
[("correctable|field1", '0'),
("correctable|field2", '0'),
Expand All @@ -218,9 +216,10 @@ def test_update_aer_to_statedb(self):
("non_fatal|field5", '0'),
("non_fatal|field6", '0'),
])
"""

daemon_pcied.update_aer_to_statedb()
assert daemon_pcied.log_debug.call_count == 1
assert daemon_pcied.device_table.set.call_count == 0
assert daemon_pcied.log_debug.call_count == 0
assert daemon_pcied.device_table.set.call_count == 1

daemon_pcied.device_table.set.reset_mock()
32 changes: 24 additions & 8 deletions sonic-pcied/tests/test_pcied.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
import pytest

# TODO: Clean this up once we no longer need to support Python 2
if sys.version_info.major == 3:
from unittest import mock
if sys.version_info >= (3, 3):
from unittest.mock import MagicMock, patch, mock_open
else:
import mock
from sonic_py_common import daemon_base, device_info
from mock import MagicMock, patch, mock_open

from sonic_py_common import daemon_base, device_info
from .mock_platform import MockPcieUtil

tests_path = os.path.dirname(os.path.abspath(__file__))
Expand All @@ -27,17 +27,33 @@
import pcied


daemon_base.db_connect = mock.MagicMock()

daemon_base.db_connect = MagicMock()

SYSLOG_IDENTIFIER = 'pcied_test'
NOT_AVAILABLE = 'N/A'


@mock.patch('pcied.load_platform_pcieutil', mock.MagicMock())
@mock.patch('pcied.DaemonPcied.run')
@patch('pcied.load_platform_pcieutil', MagicMock())
@patch('pcied.DaemonPcied.run')
def test_main(mock_run):
mock_run.return_value = False

pcied.main()
assert mock_run.call_count == 1


@patch('pcied.os.path.exists', MagicMock(return_value=True))
def test_read_id_file():

device_name = "test"

with patch('builtins.open', new_callable=mock_open, read_data='15') as mock_fd:
rc = pcied.read_id_file(device_name)
assert rc == "15"

@patch('pcied.device_info.get_paths_to_platform_and_hwsku_dirs', MagicMock(return_value=('/tmp', None)))
def test_load_platform_pcieutil():
from sonic_platform_base.sonic_pcie.pcie_common import PcieUtil
rc = pcied.load_platform_pcieutil()

assert isinstance(rc, PcieUtil)

0 comments on commit c092300

Please sign in to comment.