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

testing/ostest: add test case for task priority change with locked scheduler #1711

Merged
merged 3 commits into from Aug 8, 2023

Conversation

pkarashchenko
Copy link
Contributor

Summary

This test helps to verify that scheduler lock/unlock functionality is working as expected.
Currently there is a bug, so test fails.

Impact

More test coverage

Testing

The test pass if apache/nuttx#7464 is applied

@pkarashchenko pkarashchenko force-pushed the _ostest_priority branch 2 times, most recently from 8c2c2a3 to bf4ee4e Compare April 17, 2023 14:52
Copy link
Contributor

@davids5 davids5 left a comment

Choose a reason for hiding this comment

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

@pkarashchenko this looks fine. But I have a general comment about the os tests: I would like to see an short explanation of what the test method is and what is expected. Some times I find this lost in clever code.

@pkarashchenko
Copy link
Contributor Author

Ok. Could you please point me to some examples, or should I just place some explanations in the comments?

@pkarashchenko
Copy link
Contributor Author

Currently this is blocked by #1716

@xiaoxiang781216
Copy link
Contributor

@pkarashchenko please rebase your patch

@hartmannathan
Copy link
Contributor

Continuing what @davids5 was saying, I think a comment in the code that explains the purpose of the test and what we expect from it will help people reading the code later.

Separately, another idea:

Since the test is expected to fail due to a bug, I think we should modify the ostest program to tally five possible results from each test, like some other test suites do: pass, fail, expected fail, unexpected pass, skipped.

Explanation:

  • pass: the test passed as expected

  • fail: the test should have passed but it failed

  • expected fail: the test failed and it was expected to fail because of a known bug; that is the situation with the test in this PR. We should tally such failures separately from above-mentioned fail because they are caused by known bugs

  • unexpected pass: we expected the test to fail with "expected fail" because of a known bug, but to our surprise, it passed unexpectedly

  • skipped: the test is not being run because the hardware or build doesn't support it; e.g., the priority inheritance tests will be skipped in a non-priority-inheritance build

@jerpelea
Copy link
Contributor

jerpelea commented May 2, 2023

@pkarashchenko please add commit message describing the change

@pkarashchenko
Copy link
Contributor Author

@pkarashchenko please add commit message describing the change

Should I modify all the three commits? I'm not sure that commit that adds missing comment needs any other description than testing/ostest: add missing section comments currently available.

Copy link
Contributor

@davids5 davids5 left a comment

Choose a reason for hiding this comment

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

@pkarashchenko Thank you

@pkarashchenko
Copy link
Contributor Author

Seems like apache/nuttx#9981 is needed to move forward

@pkarashchenko pkarashchenko force-pushed the _ostest_priority branch 2 times, most recently from f05b9e3 to 81d97e4 Compare August 1, 2023 21:09
@pkarashchenko
Copy link
Contributor Author

pkarashchenko commented Aug 1, 2023

@xiaoxiang781216 do you have any ideas why sim:citest fails? Initially I thought that it is a timeout issue, but seems like it is not, I can run the tests locally and all test pass

=========================================================== test session starts ===========================================================
platform linux -- Python 3.8.10, pytest-6.2.5, py-1.11.0, pluggy-1.2.0 -- /usr/bin/python3
cachedir: .pytest_cache
rootdir: /home/homedir/nuttx/tools/ci/testrun, configfile: pytest.ini
plugins: repeat-0.9.1, ordering-0.6, json-0.4.0
collected 12 items                                                                                                                        

test_example/test_example.py::test_hello PASSED                                                                                     [  8%]
test_example/test_example.py::test_helloxx PASSED                                                                                   [ 16%]
test_example/test_example.py::test_pipe PASSED                                                                                      [ 25%]
test_example/test_example.py::test_popen PASSED                                                                                     [ 33%]
test_example/test_example.py::test_usrsocktest PASSED                                                                               [ 41%]
test_os/test_os.py::test_ostest PASSED                                                                                              [ 50%]
test_os/test_os.py::test_mm PASSED                                                                                                  [ 58%]
test_os/test_os.py::test_cxxtest PASSED                                                                                             [ 66%]
test_os/test_os.py::test_scanftest PASSED                                                                                           [ 75%]
test_os/test_os.py::test_getprime PASSED                                                                                            [ 83%]
test_os/test_os.py::test_fs_test PASSED                                                                                             [ 91%]
test_os/test_os.py::test_psram_test PASSED                                                                                          [100%]

But on CI the test_os/test_os.py::test_ostest fails.

@xiaoxiang781216
Copy link
Contributor

@ttnie could you check why the citest fail?

@pkarashchenko
Copy link
Contributor Author

@xiaoxiang781216 seems this should fix the root cause apache/nuttx#10016
But still I do not understand why it works locally for me.

@xiaoxiang781216
Copy link
Contributor

@xiaoxiang781216 seems this should fix the root cause apache/nuttx#10016 But still I do not understand why it works locally for me.

without SIM_WALLTIME_SIGNAL, simulator timing doesn't sync with the PC wall clock. I guess the ci server either run to fast or slow than your PC.

Signed-off-by: Petro Karashchenko <petro.karashchenko@gmail.com>
…heduler

Signed-off-by: Petro Karashchenko <petro.karashchenko@gmail.com>
…EEP=y

Signed-off-by: Petro Karashchenko <petro.karashchenko@gmail.com>
@xiaoxiang781216 xiaoxiang781216 merged commit 3648c0c into apache:master Aug 8, 2023
25 checks passed
@pkarashchenko pkarashchenko deleted the _ostest_priority branch August 20, 2023 11:43
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.

None yet

5 participants