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

tests: kernel/common: incorrect use of k_poll in timeout order #35275

Merged

Conversation

dcpleung
Copy link
Member

In the timeout order test, the usage of k_poll() assumes that it
only returns after all events are ready. However, that is not
the case, as k_poll() returns when non-zero number of events are
ready. This means the check for all semaphore being ready after
k_poll() will not always pass. So instead of using k_poll(),
simply wait a bit for timers to fire, then check results.

Also add some bits to clean up at the end of test.

Fixes #34585

Signed-off-by: Daniel Leung daniel.leung@intel.com

@dcpleung dcpleung requested review from andyross and nashif as code owners May 13, 2021 22:51
@github-actions github-actions bot added area: Kernel area: Tests Issues related to a particular existing or missing test labels May 13, 2021
Copy link
Member

@nashif nashif left a comment

Choose a reason for hiding this comment

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

You need to change the doxygen of this test, it says:

/**
 * @brief Test timer functionalities
 *
 * @details Test polling events with timers
 *
 * @see k_timer_start(), k_poll_event_init()
 */

In the timeout order test, the usage of k_poll() assumes that it
only returns after all events are ready. However, that is not
the case, as k_poll() returns when non-zero number of events are
ready. This means the check for all semaphore being ready after
k_poll() will not always pass. So instead of using k_poll(),
simply wait a bit for timers to fire, then check results.

Also add some bits to clean up at the end of test.

Fixes zephyrproject-rtos#34585

Signed-off-by: Daniel Leung <daniel.leung@intel.com>
@dcpleung dcpleung force-pushed the tests_kernel_common_timeout_order_fix branch from dec4cfa to 338cf9a Compare May 14, 2021 17:10
@dcpleung
Copy link
Member Author

You need to change the doxygen of this test, it says:

/**
 * @brief Test timer functionalities
 *
 * @details Test polling events with timers
 *
 * @see k_timer_start(), k_poll_event_init()
 */

Oops... updated.

@dcpleung dcpleung added this to the v2.6.0 milestone May 14, 2021
@galak galak added the bug The issue is a bug, or the PR is fixing a bug label May 19, 2021
Copy link
Collaborator

@enjiamai enjiamai left a comment

Choose a reason for hiding this comment

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

I think testing the timeout order this way is ok.

@galak galak merged commit 8fd3d18 into zephyrproject-rtos:main May 25, 2021
@dcpleung dcpleung deleted the tests_kernel_common_timeout_order_fix branch May 25, 2021 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Kernel area: Tests Issues related to a particular existing or missing test bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mec15xxevb_assy6853: test_timeout_order in tests/kernel/common assertion failed
5 participants