Skip to content

Update pcieutil error message on loading common pcie module #2786

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
May 20, 2023
Merged

Update pcieutil error message on loading common pcie module #2786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
da0c1b4
Update pcieutil load module error message
cytsao1 Apr 10, 2023
3dc2e21
Merge branch 'sonic-net:master' into pcieutil_err_msg_fix
cytsao1 Apr 10, 2023
f8a39e8
Add pcieutil test for load module warning to not print to output
cytsao1 Apr 20, 2023
b9f2b94
Merge branch 'pcieutil_err_msg_fix' of https://github.com/cytsao1/son…
cytsao1 Apr 20, 2023
5f1d7cf
Update pcieutil import test
cytsao1 May 4, 2023
b3aedb2
Update pcieutil import test
cytsao1 May 4, 2023
55af02c
Fix pcieutil import test
cytsao1 May 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pcieutil/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def load_platform_pcieutil():
from sonic_platform.pcie import Pcie
platform_pcieutil = Pcie(platform_path)
except ImportError as e:
log.log_warning("Failed to load platform Pcie module. Error : {}, fallback to load Pcie common utility.".format(str(e)), True)
log.log_warning("Failed to load platform Pcie module. Warning : {}, fallback to load Pcie common utility.".format(str(e)))
try:
from sonic_platform_base.sonic_pcie.pcie_common import PcieUtil
platform_pcieutil = PcieUtil(platform_path)
Expand Down
13 changes: 13 additions & 0 deletions tests/pcieutil_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from unittest import mock

from click.testing import CliRunner
from io import StringIO

test_path = os.path.dirname(os.path.abspath(__file__))
modules_path = os.path.dirname(test_path)
Expand Down Expand Up @@ -156,6 +157,8 @@
+---------------------+-----------+
"""

pcieutil_load_module_warning_msg = "Failed to load platform Pcie module. Warning : No module named 'sonic_platform.pcie', fallback to load Pcie common utility."

class TestPcieUtil(object):
@classmethod
def setup_class(cls):
Expand Down Expand Up @@ -199,6 +202,16 @@ def test_aer_option_device(self):
result = runner.invoke(pcieutil.cli.commands["pcie-aer"].commands["correctable"], ["-d", "0:1.0"])
assert result.output == pcieutil_pcie_aer_correctable_dev_output

def test_load_pcie_module_warning(self):
stdout = sys.stdout
sys.stdout = result = StringIO()
try:
pcieutil.load_platform_pcieutil()
except ImportError:
pass
sys.stdout = stdout
assert pcieutil_load_module_warning_msg not in result.getvalue()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cytsao1 what does this ASSERT confirm? There could be platform which may not see this import Error and in which case this ASSERT may fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is to confirm that the import warning message will not be in the console output, whether the Import error is raised or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cytsao1 result value is empty string at the start, so when the test run at line 209, result is not updated at all and at line 213 ofcourse pcieutil_load_module_warning_msg won't match result value, so i don't understand why even run pcieutil.load_platform_pcieutils() at line 209?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sys.stdout is updated to be same IO stream as result, so running load_platform_pcieutil output will be captured in result instead of output to console stdout.
After, the stdout is set back to the sys.stdout stream.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example of how it would look without this PR

>>> import sys
>>> from io import StringIO
>>> import pcieutil.main as pcieutil
>>> stdout = sys.stdout
>>> sys.stdout = result = StringIO()
>>> pcieutil.load_platform_pcieutil()
>>> sys.stdout = stdout
>>> result.getvalue()
"Failed to load platform Pcie module. Error : No module named 'sonic_platform.pcie', fallback to load Pcie common utility.\n"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cytsao1 thanks for clarifying....did not realize that stdout was captured in a string output...


@classmethod
def teardown_class(cls):
print("TEARDOWN")
Expand Down