Skip to content
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.

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

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

Merged
merged 21 commits into from
Mar 29, 2021
Merged

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

merged 21 commits into from
Mar 29, 2021

Conversation

jleveque
Copy link
Contributor

@jleveque jleveque commented Feb 20, 2021

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.

@jleveque
Copy link
Contributor Author

jleveque commented Feb 21, 2021

@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.

@jleveque
Copy link
Contributor Author

jleveque commented Feb 23, 2021

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

@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
@lgtm-com

This comment has been minimized.

@lgtm-com

This comment has been minimized.

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

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

1 similar comment
@jleveque
Copy link
Contributor Author

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

Copy link
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
lguohan pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Mar 30, 2021
Unit tests for psud depend on sonic-platform-common as of sonic-net/sonic-platform-daemons#154
daall pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Mar 31, 2021
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-buildimage that referenced this pull request May 23, 2021
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
vdahiya12 pushed a commit to vdahiya12/sonic-platform-daemons that referenced this pull request Apr 4, 2022
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants