add retry_call to test_platform_info mock check #3684
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
liat-grozovik
approved these changes
Jun 18, 2021
|
@slutati1536 Can you address the PR testing issue: |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
wangxin
requested changes
Jul 7, 2021
keboliu
reviewed
Jul 8, 2021
…o_test_platform_info_mock_check
|
@ jleveque could you please review so this PR can be merged? |
|
@keboliu , @Junchao-Mellanox and @wangxin kindly reminder please review and check if your comments were all addressed |
…o_test_platform_info_mock_check
Junchao-Mellanox
approved these changes
Aug 17, 2021
|
@wangxin could you please help to review |
wangxin
reviewed
Aug 24, 2021
| @@ -7,6 +7,7 @@ | |||
| import json | |||
| import logging | |||
| import time | |||
| from retry.api import retry_call | |||
There was a problem hiding this comment.
It looks like a new dependency is introduced here: https://pypi.org/project/retry/
Is there a way to avoid this dependency?
If not, can you update the sonic-mgmt Dockerfile in the sonic-buildimage repo to include installation of this retry package?
There was a problem hiding this comment.
Oh, I didn't notice that it has been added to the sonic-mgmt image in PR sonic-net/sonic-buildimage#7997. LGTM then.
wangxin
approved these changes
Sep 1, 2021
4 tasks
4 tasks
Blueve
pushed a commit
to Blueve/sonic-mgmt
that referenced
this pull request
Sep 9, 2021
Blueve
pushed a commit
to Blueve/sonic-mgmt
that referenced
this pull request
Sep 9, 2021
sonic-net#3684)" Revert "add retry_call to test_platform_info mock check (sonic-net#3684)" Reverted commit `734fda3d`.
wangxin
pushed a commit
that referenced
this pull request
Sep 11, 2021
What is the motivation for this PR? The test case "test_thermal_control_fan_status" will failed after the PR #3684 is merged How did you do it? Change the return value of the check_result in AbnormalFanMocker, before the #3684 is merged, it dose not care about the return value of the check_result in AbnormalFanMocker, after the PR is merged, it will check the return value of the check_result How did you verify/test it? Run test cases in test_platform_info.py and all the test cases pass
vmittal-msft
pushed a commit
to vmittal-msft/sonic-mgmt
that referenced
this pull request
Sep 28, 2021
vmittal-msft
pushed a commit
to vmittal-msft/sonic-mgmt
that referenced
this pull request
Sep 28, 2021
What is the motivation for this PR? The test case "test_thermal_control_fan_status" will failed after the PR sonic-net#3684 is merged How did you do it? Change the return value of the check_result in AbnormalFanMocker, before the sonic-net#3684 is merged, it dose not care about the return value of the check_result in AbnormalFanMocker, after the PR is merged, it will check the return value of the check_result How did you verify/test it? Run test cases in test_platform_info.py and all the test cases pass
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of PR
the following cases:
are executing a command once and comparing the output to the expected mock data,
sometimes differences between the mock and the actual are causing the tests to fail. retry will make these tests more robust since those failures are very hard to reproduce and also they rarely happen.
Summary:
add retries to test in order to make them more stable
Type of change
Back port request
Approach
What is the motivation for this PR?
those cases sometimes fail because of mock data mismatch, for example:
actual: {
},
expected:
"PSU-1 Temp": {
},
retry will compare the actual info several times before failing, making the tests more stable.
How did you do it?
add my changes and tested them locally to check nothing was broken
How did you verify/test it?
run the cases locally
Any platform specific information?
no