Skip to content

timeout_tests: do not call sleep from test thread #10698

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

Merged

Conversation

LMESTM
Copy link
Contributor

@LMESTM LMESTM commented May 29, 2019

Description

This is up to scheduler to call sleep when threads are inactive.
So the thread should simply wait for the semaphore for ever (or test timeout).

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[x ] Test update
[ ] Breaking change

Reviewers

@jeromecoutant @mprse

Release Notes

@ciarmcom ciarmcom requested review from mprse and a team May 29, 2019 09:00
@ciarmcom
Copy link
Member

@LMESTM, thank you for your changes.
@mprse @ARMmbed/mbed-os-core @ARMmbed/mbed-os-test @ARMmbed/mbed-os-maintainers please review.

@kjbracey
Copy link
Contributor

Indeed, but I'm wondering if there is some subtle reason for the code being like that I'm missing.

Following the Semaphore API revision just made, Semaphore::wait should be Semaphore::acquire.

@kjbracey
Copy link
Contributor

Is this fixing a particular issue you've spotted?

@LMESTM
Copy link
Contributor Author

LMESTM commented May 29, 2019

@kjbracey-arm I'm working on updating LP ticker implementation for STM targets. The changes have highlighted some dependencies to test updates. I'm pushing generic test updates first, and will reference them from the LP ticker change.

According to your comment, should I replace wait by acquire in this PR ?

@kjbracey
Copy link
Contributor

kjbracey commented May 29, 2019

Yes, wait has just been deprecated - the code you're changing has just been changed from wait(0) to try_acquire().

@LMESTM
Copy link
Contributor Author

LMESTM commented May 29, 2019

#10701 is the change that triggered this test update
I will update the PR with new API

This is up to scheduler to call sleep when threads are inactive.
So the thread should simply wait for the semaphore for ever (or test timeout).
@LMESTM LMESTM force-pushed the Timeout_test_do_not_call_sleep_from_thread branch from afe9881 to 19b843d Compare May 29, 2019 11:08
@LMESTM
Copy link
Contributor Author

LMESTM commented May 29, 2019

PR updated with new Semaphore API and verified on NUCLEO_L476RG

+---------------------+---------------+------------------------------+-------------------------------------------------+--------+--------+--------+--------------------+
| target              | platform_name | test suite                   | test case                                       | passed | failed | result | elapsed_time (sec) |
+---------------------+---------------+------------------------------+-------------------------------------------------+--------+--------+--------+--------------------+
| NUCLEO_L476RG-ARMC6 | NUCLEO_L476RG | tests-mbed_hal-sleep_manager | deep sleep lock/unlock                          | 1      | 0      | OK     | 0.05               |
| NUCLEO_L476RG-ARMC6 | NUCLEO_L476RG | tests-mbed_hal-sleep_manager | deep sleep lock/unlock test_check               | 1      | 0      | OK     | 0.07               |
| NUCLEO_L476RG-ARMC6 | NUCLEO_L476RG | tests-mbed_hal-sleep_manager | deep sleep locked USHRT_MAX times               | 1      | 0      | OK     | 0.12               |
| NUCLEO_L476RG-ARMC6 | NUCLEO_L476RG | tests-mbed_hal-sleep_manager | sleep_auto calls sleep/deep sleep based on lock | 1      | 0      | OK     | 0.14               |
+---------------------+---------------+------------------------------+-------------------------------------------------+--------+--------+--------+--------------------+

@0xc0170
Copy link
Contributor

0xc0170 commented May 31, 2019

CI started while waiting for more reviews

@mbed-ci
Copy link

mbed-ci commented May 31, 2019

Test run: FAILED

Summary: 3 of 3 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-GCC_ARM
  • jenkins-ci/mbed-os-ci_build-IAR
  • jenkins-ci/mbed-os-ci_build-ARM

@mbed-ci
Copy link

mbed-ci commented May 31, 2019

Test run: SUCCESS

Summary: 4 of 4 test jobs passed
Build number : 2
Build artifacts

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.

6 participants