Retry test_psu.py::TestPsuApi::test_power if power not within tolerance #14788

smagarwal-arista · 2024-09-27T16:39:10Z

Description of PR

Summary:
If test_psu.py::TestPsuApi::test_power fails on abs(power - (voltage*current)) < power*0.1 then retry the test. The test is repeated for a maximum of three times until it passes, else it is failed. This change is introduced to account for the stringent tolerance level of 10% and the errors that might be caused due to time differences in reading each of the parameters (current, voltage and power).

Fixes # (issue)
Addresses issue: https://github.com/aristanetworks/sonic-qual.msft/issues/209

Type of change

Bug fix
Testbed and Framework(new/improvement)
Test case(new/improvement)

Back port request

Approach

What is the motivation for this PR?

Improve the test to account for stringent tolerance level and asymmetric reading of parameters for power calculation.

How did you do it?

If the test fails for the condition abs(power - (voltage*current)) < power*0.1, the test will collect fresh parameters (voltage, current and power) from the PSU telemetry and recheck the condition. It is limited to a maximum of 3 attempts.

How did you verify/test it?

Tested internally by varying the tolerance level and verifying the number of api calls.

Any platform specific information?

N/A

Supported testbed topology if it's a new test case?

N/A

Documentation

N/A

If the test fails on `abs(power - (voltage*current)) < power*0.1` then retry the test. Test is repeated for a maximum of three times until it passes, else it is failed. This change is introduced to account for the stringent tolerance level of 10% and the errors that might be caused due to time differences reading each of the parameters.

mssonicbld · 2024-09-27T16:39:53Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed - hook id: trailing-whitespace - exit code: 1 - files were modified by this hook Fixing tests/platform_tests/api/test_psu.py fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/platform_tests/api/test_psu.py:210:29: E231 missing whitespace after ',' tests/platform_tests/api/test_psu.py:238:37: E128 continuation line under-indented for visual indent tests/platform_tests/api/test_psu.py:252:33: E128 continuation line under-indented for visual indent tests/platform_tests/api/test_psu.py:256:121: E501 line too long (122 > 120 characters) tests/platform_tests/api/test_psu.py:260:37: E128 continuation line under-indented for visual indent ... [truncated extra lines, please run pre-commit locally to view full check results]

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

If the test fails on `abs(power - (voltage*current)) < power*0.1` then retry the test. The test is repeated for a maximum of three times until it passes. This change is introduced to account for the stringent tolerance level of 10% and the errors that might be caused due to time differences in reading each of the parameters.

smagarwal-arista · 2024-09-30T18:04:53Z

/azpw run Azure.sonic-mgmt

mssonicbld · 2024-09-30T18:04:56Z

/AzurePipelines run Azure.sonic-mgmt

azure-pipelines · 2024-09-30T18:05:07Z

Azure Pipelines successfully started running 1 pipeline(s).

vvolam · 2024-10-01T17:06:06Z

tests/platform_tests/api/test_psu.py

+                                    .format(voltage, psu_id, low_threshold, high_threshold))
+
+                break
+
        self.assert_expectations()


How does the self.assert_expectations() evaluate the 3 test run values? What happens for different combinations of the expected values? For example:

Test1: True
Test2: False
Test3: True

In this combination, does the test raise assert or passes?

The test would fail in this scenario. I have an update to address it, will commit the changes. Thanks.

@smagarwal-arista With new changes, does the test pass in above scenario? Also, does the test exit on first Passed case?

Yes, the test would exit the retry loop if all expects pass. Retry is only targeted for expect failing on abs(power - (voltage*current)) < power*0.1 condition. Any other expect failures would exit the retry loop for that PSU and eventually result in a failed test.

mssonicbld · 2024-10-08T15:36:55Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed - hook id: trailing-whitespace - exit code: 1 - files were modified by this hook Fixing tests/platform_tests/api/test_psu.py fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Passed flake8...............................................(no files to check)Skipped check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

smagarwal-arista · 2024-10-11T22:15:50Z

/azpw run Azure.sonic-mgmt

mssonicbld · 2024-10-11T22:15:53Z

/AzurePipelines run Azure.sonic-mgmt

azure-pipelines · 2024-10-11T22:16:03Z

Azure Pipelines successfully started running 1 pipeline(s).

vvolam

Looks good to me.

@smagarwal-arista As we are modifying this function already, can you modularize the function for better readability?

smagarwal-arista · 2024-10-14T19:08:38Z

Looks good to me.

@smagarwal-arista As we are modifying this function already, can you modularize the function for better readability?

Sure, will update the function

vvolam

Thank you. Looks good to me.

vvolam · 2024-10-17T16:39:28Z

@prgeor could you review the PR?

prgeor

@roy-sror @keboliu @Junchao-Mellanox can you review

prgeor

@Junchao-Mellanox @keboliu @roy-sror can you review?

JibinBao

In the case, you want to retry the same test logic for 3 times. I think you don't need to desgin the retry mechanisam by yourself, you can use the exsiting function:
wait_until(timeout, interval, delay, condition, *args, **kwargs).
Using the function of wait_until will make the test more concise

JibinBao · 2024-10-29T02:15:05Z

tests/platform_tests/api/test_psu.py

+                    power = self.get_psu_parameter(psu_info, "power", psu.get_power, "power")
+
+                    failure_occured = self.get_len_failed_expectations() > failure_count
+                    if current is not None and voltage is not None and power is not None:


'if current is not None and voltage is not None and power is not None: ' is changed to if current and voltage and power :
Maybe It is more concise?

Resolved in the latest commit.

JibinBao · 2024-10-29T02:20:10Z

tests/platform_tests/api/test_psu.py

+                    low_threshold = self.get_psu_parameter(psu_info, "voltage_low_threshold",
+                                                           psu.get_voltage_low_threshold, "low voltage threshold")
+
+                    if high_threshold is not None and low_threshold is not None:


Same as line 236. if high_threshold and low_threshold:

Resolved in the latest commit.

smagarwal-arista · 2024-10-30T18:25:06Z

In the case, you want to retry the same test logic for 3 times. I think you don't need to desgin the retry mechanisam by yourself, you can use the exsiting function: wait_until(timeout, interval, delay, condition, *args, **kwargs). Using the function of wait_until will make the test more concise

Thanks, I'll take a look

StormLiangMS · 2024-11-07T06:10:44Z

hi @smagarwal-arista could you address the comments?

smagarwal-arista · 2024-11-07T06:29:08Z

In the case, you want to retry the same test logic for 3 times. I think you don't need to desgin the retry mechanisam by yourself, you can use the exsiting function: wait_until(timeout, interval, delay, condition, *args, **kwargs). Using the function of wait_until will make the test more concise

Resolved in the latest commit. Now using wait_until function.

…ce (sonic-net#14788) * Retry test_power calculated power above 10% If the test fails on `abs(power - (voltage*current)) < power*0.1` then retry the test. Test is repeated for a maximum of three times until it passes, else it is failed. This change is introduced to account for the stringent tolerance level of 10% and the errors that might be caused due to time differences reading each of the parameters. * Retry test_power.py::TestPsuApi::test_power if power above tolerance If the test fails on `abs(power - (voltage*current)) < power*0.1` then retry the test. The test is repeated for a maximum of three times until it passes. This change is introduced to account for the stringent tolerance level of 10% and the errors that might be caused due to time differences in reading each of the parameters. * Add check to detect occurrence of a failure before power calculation * Resolve pre-commit check issue * Refactor test_power function to improve readability * Use wait_until function for retry

mssonicbld · 2024-11-07T23:48:44Z

Cherry-pick PR to 202405: #15451

bingwang-ms · 2024-11-07T23:49:04Z

MSADO: 28386639

…ce (#14788) * Retry test_power calculated power above 10% If the test fails on `abs(power - (voltage*current)) < power*0.1` then retry the test. Test is repeated for a maximum of three times until it passes, else it is failed. This change is introduced to account for the stringent tolerance level of 10% and the errors that might be caused due to time differences reading each of the parameters. * Retry test_power.py::TestPsuApi::test_power if power above tolerance If the test fails on `abs(power - (voltage*current)) < power*0.1` then retry the test. The test is repeated for a maximum of three times until it passes. This change is introduced to account for the stringent tolerance level of 10% and the errors that might be caused due to time differences in reading each of the parameters. * Add check to detect occurrence of a failure before power calculation * Resolve pre-commit check issue * Refactor test_power function to improve readability * Use wait_until function for retry

…ce (sonic-net#14788) * Retry test_power calculated power above 10% If the test fails on `abs(power - (voltage*current)) < power*0.1` then retry the test. Test is repeated for a maximum of three times until it passes, else it is failed. This change is introduced to account for the stringent tolerance level of 10% and the errors that might be caused due to time differences reading each of the parameters. * Retry test_power.py::TestPsuApi::test_power if power above tolerance If the test fails on `abs(power - (voltage*current)) < power*0.1` then retry the test. The test is repeated for a maximum of three times until it passes. This change is introduced to account for the stringent tolerance level of 10% and the errors that might be caused due to time differences in reading each of the parameters. * Add check to detect occurrence of a failure before power calculation * Resolve pre-commit check issue * Refactor test_power function to improve readability * Use wait_until function for retry

mssonicbld · 2024-11-15T17:11:47Z

Cherry-pick PR to 202311: #15587

smagarwal-arista closed this Sep 27, 2024

smagarwal-arista reopened this Sep 27, 2024

smagarwal-arista changed the title ~~Retry test_power calculated power above 10%~~ Retry test_psu.py::TestPsuApi::test_power if power not within tolerance Sep 27, 2024

smagarwal-arista marked this pull request as ready for review October 1, 2024 13:40

smagarwal-arista requested a review from prgeor as a code owner October 1, 2024 13:40

vvolam reviewed Oct 1, 2024

View reviewed changes

Add check to detect occurrence of a failure before power calculation

caacd06

Resolve pre-commit check issue

6928d80

vvolam reviewed Oct 11, 2024

View reviewed changes

Refactor test_power function to improve readability

8b85062

vvolam approved these changes Oct 17, 2024

View reviewed changes

prgeor reviewed Oct 17, 2024

View reviewed changes

keboliu requested review from roy-sror and nhe-NV October 18, 2024 01:52

prgeor reviewed Oct 25, 2024

View reviewed changes

JibinBao reviewed Oct 29, 2024

View reviewed changes

Use wait_until function for retry

c52c7a7

prgeor approved these changes Nov 7, 2024

View reviewed changes

prgeor merged commit db8b517 into sonic-net:master Nov 7, 2024
15 checks passed

vvolam added Request for 202311 branch Request for 202405 branch labels Nov 7, 2024

bingwang-ms added the Approved for 202405 branch label Nov 7, 2024

mssonicbld added the Created PR to 202405 branch label Nov 7, 2024

mssonicbld mentioned this pull request Nov 7, 2024

[action] [PR:14788] Retry test_psu.py::TestPsuApi::test_power if power not within tolerance #15451

Merged

8 tasks

mssonicbld added Included in 202405 branch and removed Created PR to 202405 branch labels Nov 8, 2024

yxieca added the Approved for 202311 branch label Nov 15, 2024

mssonicbld added the Created PR to 202311 branch label Nov 15, 2024

mssonicbld mentioned this pull request Nov 15, 2024

[action] [PR:14788] Retry test_psu.py::TestPsuApi::test_power if power not within tolerance #15587

Open

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Retry test_psu.py::TestPsuApi::test_power if power not within tolerance #14788

Retry test_psu.py::TestPsuApi::test_power if power not within tolerance #14788

smagarwal-arista commented Sep 27, 2024 •

edited

Loading

mssonicbld commented Sep 27, 2024

smagarwal-arista commented Sep 30, 2024

mssonicbld commented Sep 30, 2024

azure-pipelines bot commented Sep 30, 2024

vvolam Oct 1, 2024

smagarwal-arista Oct 7, 2024

vvolam Oct 11, 2024

smagarwal-arista Oct 11, 2024

mssonicbld commented Oct 8, 2024

smagarwal-arista commented Oct 11, 2024

mssonicbld commented Oct 11, 2024

azure-pipelines bot commented Oct 11, 2024

vvolam left a comment

smagarwal-arista commented Oct 14, 2024

vvolam left a comment

vvolam commented Oct 17, 2024

prgeor left a comment

prgeor left a comment

JibinBao left a comment

JibinBao Oct 29, 2024

smagarwal-arista Nov 7, 2024

JibinBao Oct 29, 2024

smagarwal-arista Nov 7, 2024

smagarwal-arista commented Oct 30, 2024

StormLiangMS commented Nov 7, 2024

smagarwal-arista commented Nov 7, 2024

mssonicbld commented Nov 7, 2024

bingwang-ms commented Nov 7, 2024

mssonicbld commented Nov 15, 2024

Retry test_psu.py::TestPsuApi::test_power if power not within tolerance #14788

Retry test_psu.py::TestPsuApi::test_power if power not within tolerance #14788

Conversation

smagarwal-arista commented Sep 27, 2024 • edited Loading

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

mssonicbld commented Sep 27, 2024

smagarwal-arista commented Sep 30, 2024

mssonicbld commented Sep 30, 2024

azure-pipelines bot commented Sep 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mssonicbld commented Oct 8, 2024

smagarwal-arista commented Oct 11, 2024

mssonicbld commented Oct 11, 2024

azure-pipelines bot commented Oct 11, 2024

vvolam left a comment

Choose a reason for hiding this comment

smagarwal-arista commented Oct 14, 2024

vvolam left a comment

Choose a reason for hiding this comment

vvolam commented Oct 17, 2024

prgeor left a comment

Choose a reason for hiding this comment

prgeor left a comment

Choose a reason for hiding this comment

JibinBao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smagarwal-arista commented Oct 30, 2024

StormLiangMS commented Nov 7, 2024

smagarwal-arista commented Nov 7, 2024

mssonicbld commented Nov 7, 2024

bingwang-ms commented Nov 7, 2024

mssonicbld commented Nov 15, 2024

smagarwal-arista commented Sep 27, 2024 •

edited

Loading