Skip to content

[psud] Increase unit test coverage; Refactor mock platform#154

Merged
jleveque merged 21 commits intosonic-net:masterfrom
jleveque:increase_psud_coverage
Mar 29, 2021
Merged

[psud] Increase unit test coverage; Refactor mock platform#154
jleveque merged 21 commits intosonic-net:masterfrom
jleveque:increase_psud_coverage

Conversation

@jleveque
Copy link
Copy Markdown
Contributor

@jleveque jleveque commented Feb 20, 2021 •
edited
Loading

Description

  • Refactor psud such that the run() method does not contain an infinite loop, thus allowing us to unit test it
  • Refactor mock_platform.py such that it inherits from sonic-platform-common in order to ensure it is aligned with the current API definitions (this introduces a test-time dependency on the sonic-platform-common package)
  • Eliminate the need to check for a PSUD_UNIT_TESTING environment variable in daemon code
  • Increase overall unit test unit test coverage from 78% to 97%

Previous unit test coverage:

----------- coverage: platform linux, python 3.7.3-final-0 -----------
Name           Stmts   Miss  Cover
----------------------------------
scripts/psud     360     81    78%
Coverage HTML written to dir htmlcov
Coverage XML written to file coverage.xml

Unit test coverage with this patch:

----------- coverage: platform linux, python 3.7.3-final-0 -----------
Name           Stmts   Miss  Cover
----------------------------------
scripts/psud     366     12    97%
Coverage HTML written to dir htmlcov
Coverage XML written to file coverage.xml

Motivation and Context

To increase unit test coverage >= 90% in order to prevent regressions

How Has This Been Tested?

The refactored psud has been tested on a physical DUT and the unit test results can be examined via the Azure Pipelines check builds in this PR.

@jleveque jleveque self-assigned this Feb 20, 2021
@jleveque jleveque changed the title [psud] Increase unit test coverage [psud] Increase unit test coverage; Refactor mock platform Feb 20, 2021
@lgtm-com

This comment has been minimized.

Sign in to view
@jleveque
Copy link
Copy Markdown
Contributor Author

jleveque commented Feb 21, 2021 •
edited
Loading

@mprabhu-nokia: I have done a bit more refactoring to psud in order to increase unit test coverage. Can you please test and review these changes on a modular chassis?

Update: Please wait on testing and reviewing these changes until the sonic-platform-common PR merges and I get the check builds passing. I will notify you again at that point.

@lgtm-com

This comment has been minimized.

Sign in to view
@jleveque
Copy link
Copy Markdown
Contributor Author

jleveque commented Feb 23, 2021 •
edited
Loading

Unit test is failing due to a bug uncovered in sonic_platform_base. PsuBase.psu_master_led_color is defined as a class attribute and then also defined as an instance attribute in the initializer, which takes precedence. Fix here: sonic-net/sonic-platform-common#173

mprabhu-nokia
mprabhu-nokia reviewed Feb 23, 2021
View reviewed changes
@sonic-net sonic-net deleted a comment from azure-pipelines bot Mar 10, 2021
@sonic-net sonic-net deleted a comment from azure-pipelines bot Mar 12, 2021
Joe LeVeque added 19 commits March 17, 2021 23:26
[psud] Increase unit test coverage
2a1cf59
Mock Platform inherits from sonic_platform_base
033a40c
Add test case for _update_single_psu_data()
df3e544
Refactor psud to eliminate inifinte loop from run()
62237d8
Update tests to align with refactored psud
92b6cb8
Add unit test for run()
c9add2a
Use os.path.join() to create paths
10f9149
[pytest] Increase verbosity
b35d18e
Add 'mocked_libs' directory, remove need for PSUD_UNIT_TESTING env var
c9509da
Fix test_power_budget()
c016f3d
Enhance test_power_budget() to better test class attribute
5658371
[TestPsuChassisInfo] Fix variable names
fd3db3e
Refactor signal handler; ensure exit with non-zero code
460843a
Psu.set_status_master_led() will never raise NotImplementedError
907585d
Add TODO comments
21e576f
Use autopep8
2b4a4bf
Use .mock_calls for mutiple calls
586123a
Clean up mock_platform.py
9152ba0
Add underscore prefixes to private class members
c7b5da2
@lgtm-com

This comment has been minimized.

Sign in to view
@lgtm-com

This comment has been minimized.

Sign in to view
@jleveque jleveque marked this pull request as ready for review March 18, 2021 02:20
@jleveque
Copy link
Copy Markdown
Contributor Author

jleveque commented Mar 18, 2021

@mprabhu-nokia: Can you please review and test this PR?

1 similar comment
@jleveque
Copy link
Copy Markdown
Contributor Author

jleveque commented Mar 26, 2021

@mprabhu-nokia: Can you please review and test this PR?

mprabhu-nokia
mprabhu-nokia approved these changes Mar 29, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@mprabhu-nokia mprabhu-nokia left a comment

Choose a reason for hiding this comment

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

Looks good.

@jleveque jleveque merged commit c5be3ca into sonic-net:master Mar 29, 2021
@jleveque jleveque deleted the increase_psud_coverage branch March 29, 2021 17:37
@jleveque jleveque mentioned this pull request Mar 30, 2021
Merged
4 tasks
lguohan pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Mar 30, 2021
@jleveque
[sonic-psud] Depend on sonic-platform-common (#7182)
9dd45da 
Unit tests for psud depend on sonic-platform-common as of sonic-net/sonic-platform-daemons#154
@stephenxs stephenxs mentioned this pull request Mar 31, 2021
Merged
4 tasks
daall pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Mar 31, 2021
@jleveque @daall
[sonic-psud] Depend on sonic-platform-common (#7182)
35f2924 
Unit tests for psud depend on sonic-platform-common as of sonic-net/sonic-platform-daemons#154
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-buildimage that referenced this pull request May 23, 2021
@jleveque @raphaelt-nvidia
[sonic-psud] Depend on sonic-platform-common (sonic-net#7182)
ab5eee4 
Unit tests for psud depend on sonic-platform-common as of sonic-net/sonic-platform-daemons#154
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
@jleveque
[sonic-psud] Depend on sonic-platform-common (sonic-net#7182)
f5aeb8f 
Unit tests for psud depend on sonic-platform-common as of sonic-net/sonic-platform-daemons#154
vdahiya12 pushed a commit to vdahiya12/sonic-platform-daemons that referenced this pull request Apr 4, 2022
@Junchao-Mellanox
[y_cable] Skip ImportError in y_cable.py (sonic-net#154)
7e23e63 
When build python3 xcvrd, it tries to do basic check which will import this y_cable.py. However, not all platform supports python3 API now, so it could cause an issue when importing sonic_platform.platform

We skip the ImportError here to make the builder happy. And this is safe because:
- If any python package is not available, there will be exception when use it
- Vendors know their platform API version, they are responsible to use correct python version when importing this file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@mprabhu-nokia mprabhu-nokia mprabhu-nokia approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jleveque jleveque

Labels

psud Unit Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jleveque @mprabhu-nokia