Skip to content

Conversation

@jkmar
Copy link
Contributor

@jkmar jkmar commented May 6, 2025

The test_execute_reboot_fail_halt_timeout test was very slow - it took around a minute:

user@host:/sonic/src/sonic-host-services$ pytest-3 tests/host_modules/reboot_test.py::TestReboot::test_execute_reboot_fail_halt_timeout
=============================================================== test session starts ================================================================
platform linux -- Python 3.9.2, pytest-6.0.2, py-1.10.0, pluggy-0.13.0 -- /usr/bin/python3
cachedir: .pytest_cache
rootdir: /sonic/src/sonic-host-services, configfile: pytest.ini
plugins: pyfakefs-5.2.4, cov-2.10.1
collected 1 item

tests/host_modules/reboot_test.py::TestReboot::test_execute_reboot_fail_halt_timeout PASSED                                                  [100%]
...
===================================================== 1 passed, 3 warnings in 60.44s (0:01:00) =====================================================

The reason is that within this while loop: https://github.com/sonic-net/sonic-host-services/blob/master/host_modules/reboot.py#L131, only sleep in the body was mocked, but the condition wasn't, which caused the loop to be run continuously until the halt timer expired.

This PR fixes the issue, by mocking the monotonic clock used in the loop's condition, so that it ends earlier.
Now the test takes much less time:

user@host:/sonic/src/sonic-host-services$ pytest-3 tests/host_modules/reboot_test.py::TestReboot::test_execute_reboot_fail_halt_timeout
...
========================================================== 1 passed, 3 warnings in 0.46s ===========================================================

@mssonicbld
Copy link

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jkmar jkmar marked this pull request as ready for review May 7, 2025 09:17
@jkmar
Copy link
Contributor Author

jkmar commented May 14, 2025

Copy link

@anders-nexthop anders-nexthop left a comment

Choose a reason for hiding this comment

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

This change looks fine to me, and the test is passing. @jkmar you will have better luck getting visibility into PRs if you create a corresponding issue for them; individual PRs do not get looked at very often if at all, whereas issues are generally triaged every couple of weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants