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

Skip Greentea tests for Mbed OS code coverage on Fast Models #7805

Merged
merged 6 commits into from
Aug 27, 2018

Conversation

jamesbeyond
Copy link
Contributor

Description

This PR updated the Greentea tests which failed on code coverage testing on FastModels.

  • Skip time drifting test on FastModels targets
    FastModels targets are simulator running on the x86 hosts.
    As the nature of non-RealTime x86 OS and FastModels, timing accuracy is not guaranteed
    So skipping the time drifting tests on FastModel targets

  • Increase the thread stack size on FastModel targets
    The thread stack size was restricted due to some boards have really limited RAM sizes,
    and out of heap memory on multiple threads tests.
    The side effect was on the debug profile build, the tests will get stack overflow.
    We need the build the test with debug profile in order to do the code coverage analysis.
    So increased the thread stack size on FastModel targets.

This PR have a dependency on #7706

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Feature
[ ] Breaking change

Qinghao Shi added 4 commits August 17, 2018 17:46
FastModels targets are simulator running on the x86 hosts.
As the nature of non-RealTime x86 OS and FastModels, timing accuracy is not guaranteed
So skipping the time drifting tests on FastModel targets
The thread stack size was restricted due to some boards have really limited RAM sizes,
and out of heap memory on multiple threads tests.
The side effect was on the debug profile build, the tests will get stack overflow.
We need the build the test with debug profile in order to do the code coverage analysis.
So increased the thread stack size on FastModel targets.
@jamesbeyond
Copy link
Contributor Author

Here attached the greentea test log results for reference:

+-----------------+---------------+------------------------------------------------------------------------------+--------+--------------------+-------------+
| target          | platform_name | test suite                                                                   | result | elapsed_time (sec) | copy_method |
+-----------------+---------------+------------------------------------------------------------------------------+--------+--------------------+-------------+
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-device_key-tests-device_key-functionality                           | OK     | 9.18               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-frameworks-utest-tests-unit_tests-basic_test                        | OK     | 9.0                | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-frameworks-utest-tests-unit_tests-basic_test_default                | OK     | 9.43               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-frameworks-utest-tests-unit_tests-case_async_validate               | OK     | 8.24               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-frameworks-utest-tests-unit_tests-case_control_async                | OK     | 11.26              | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-frameworks-utest-tests-unit_tests-case_control_repeat               | OK     | 8.06               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-frameworks-utest-tests-unit_tests-case_selection                    | OK     | 8.01               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-frameworks-utest-tests-unit_tests-case_setup_failure                | OK     | 8.02               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-frameworks-utest-tests-unit_tests-case_teardown_failure             | OK     | 8.15               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-frameworks-utest-tests-unit_tests-control_type                      | OK     | 8.89               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-frameworks-utest-tests-unit_tests-minimal_async_scheduler           | OK     | 8.79               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-frameworks-utest-tests-unit_tests-minimal_scheduler                 | OK     | 8.78               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-frameworks-utest-tests-unit_tests-test_assertion_failure_test_setup | OK     | 8.73               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-frameworks-utest-tests-unit_tests-test_setup_case_selection_failure | OK     | 9.02               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-frameworks-utest-tests-unit_tests-test_setup_failure                | OK     | 8.54               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-frameworks-utest-tests-unit_tests-test_skip                         | OK     | 8.68               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-nvstore-tests-nvstore-functionality                                 | OK     | 9.98               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-tests-filesystem-buffered_block_device                              | OK     | 8.56               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-tests-filesystem-flashsim_block_device                              | OK     | 9.07               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-tests-filesystem-heap_block_device                                  | OK     | 9.19               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-tests-filesystem-mbr_block_device                                   | OK     | 7.94               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | features-tests-filesystem-util_block_device                                  | OK     | 7.98               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-events-queue                                                           | OK     | 11.85              | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-events-timing                                                          | OK     | 46.19              | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-integration-basic                                                      | OK     | 8.96               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_drivers-c_strings                                                 | OK     | 8.53               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_drivers-crc                                                       | OK     | 8.27               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_drivers-dev_null                                                  | OK     | 10.9               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_drivers-echo                                                      | OK     | 19.8               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_drivers-flashiap                                                  | OK     | 10.85              | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_drivers-generic_tests                                             | OK     | 8.05               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_drivers-race_test                                                 | OK     | 8.95               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_drivers-stl_features                                              | OK     | 7.99               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_drivers-ticker                                                    | OK     | 9.11               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_drivers-timeout                                                   | OK     | 9.8                | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_drivers-timer                                                     | OK     | 9.47               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_drivers-timerevent                                                | OK     | 8.25               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_functional-callback                                               | OK     | 7.96               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_functional-callback_big                                           | OK     | 7.98               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_functional-callback_small                                         | OK     | 8.75               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_functional-functionpointer                                        | OK     | 8.99               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_hal-common_tickers                                                | OK     | 7.97               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_hal-common_tickers_freq                                           | OK     | 8.98               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_hal-critical_section                                              | OK     | 8.8                | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_hal-flash                                                         | OK     | 8.07               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_hal-rtc_time                                                      | OK     | 8.15               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_hal-rtc_time_conv                                                 | OK     | 117.25             | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_hal-ticker                                                        | OK     | 13.72              | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_hal-us_ticker                                                     | OK     | 8.18               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_platform-critical_section                                         | OK     | 8.02               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_platform-error_handling                                           | OK     | 9.09               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_platform-filehandle                                               | OK     | 8.56               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_platform-singletonptr                                             | OK     | 8.13               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_platform-system_reset                                             | OK     | 10.16              | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbed_platform-transaction                                              | OK     | 8.75               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-mbed-attributes                                              | OK     | 8.8                | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-mbed-call_before_main                                        | OK     | 8.05               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-mbed-cpp                                                     | OK     | 8.78               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-mbed-div                                                     | OK     | 7.94               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-mbed-static_assert                                           | OK     | 7.94               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-rtos-mbed-basic                                              | OK     | 9.1                | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-rtos-mbed-circularbuffer                                     | OK     | 9.85               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-rtos-mbed-condition_variable                                 | OK     | 11.61              | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-rtos-mbed-event_flags                                        | OK     | 8.94               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-rtos-mbed-heap_and_stack                                     | OK     | 9.53               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-rtos-mbed-kernel_tick_count                                  | OK     | 9.26               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-rtos-mbed-mail                                               | OK     | 8.36               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-rtos-mbed-malloc                                             | OK     | 16.49              | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-rtos-mbed-memorypool                                         | OK     | 11.82              | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-rtos-mbed-mutex                                              | OK     | 11.02              | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-rtos-mbed-queue                                              | OK     | 9.2                | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-rtos-mbed-rtostimer                                          | OK     | 8.92               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-rtos-mbed-semaphore                                          | OK     | 9.02               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-rtos-mbed-signals                                            | OK     | 8.1                | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedmicro-rtos-mbed-threads                                            | OK     | 9.12               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedtls-multi                                                          | OK     | 9.33               | default     |
| FVP_MPS2_M0-ARM | FVP_MPS2_M0   | tests-mbedtls-selftest                                                       | OK     | 9.92               | default     |
+-----------------+---------------+------------------------------------------------------------------------------+--------+--------------------+-------------+
mbedgt: test suite results: 77 OK
mbedgt: test case report:

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 20, 2018

This PR have a dependency on #7706

Merged. This is ready for review!

@cmonr cmonr changed the title Fix the Greentea tests for Mbed OS code coverage on Fast Models Skip Greentea tests for Mbed OS code coverage on Fast Models Aug 21, 2018
@@ -104,6 +104,10 @@ void increment_multi_counter(void)
*/
void test_case_1x_ticker()
{
#if defined(__ARM_FM)
Copy link
Contributor

Choose a reason for hiding this comment

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

If it's required to skip tests, the pattern that we use is to add the #if def in the Cases cases[] array.
This makes determining if tests are being skipped much easier to follow.

Please make this change to this and the other tests.

Copy link
Contributor Author

@jamesbeyond jamesbeyond Aug 21, 2018

Choose a reason for hiding this comment

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

Actually, that was my first attempt. with more then one case it just fine.
However, some of the tests only have one test case, e.g. here
I can't skip the only test case in the Case case[ ] . any suggestions on this case ? should I block the whole test file as NOT_SUPPORTTED

I took the currently method purely becasue it give a message while skipping the tests. I admit it not as easy as the other on to follow in the code level.
@cmonr

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, that's a good point.

As an example for how things are done right now (doesn't mean that they should be like this, just means that this is how we do it right now), take a look at this test: https://github.com/ARMmbed/mbed-os/blob/master/TESTS/mbed_hal/common_tickers/main.cpp

There's an ifdef on the top of the file for targets that don't have the supported feature, and another ifdef for if a different feature is available. Please follow this pattern for this PR, and I would appreciate an issue being opened for this alternate method.

@jamesbeyond
Copy link
Contributor Author

@cmonr please have another look

Case("Test timers: 1x ticker", test_case_1x_ticker),
Case("Test timers: 2x ticker", test_case_2x_ticker)
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

if test_case_2x_ticker is skipped from compiled time do we need runtime check on line 149 ?

What limitations are there for ARM_FM for this 2 test cases ? I would like to avoid having this ifdef there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. my bad forgot to remove line 149, which suggested by @cmonr.

The reason was stated in the PR description:

FastModels targets are simulator running on the x86 hosts.
As the nature of non-RealTime x86 OS and FastModels, timing accuracy is not guaranteed
mbed time-drifting-tests will failing all the time on FastModel targets

my first version to skip the test was in line 149, now it been changed to like line 338

@jamesbeyond
Copy link
Contributor Author

@cmonr, Thanks for pointing out, my case is a bit different though. putting #ifdef macro on the top is not ideal, and keep the test file from being built.
But this it is easy to understand the which test got skipped, compare put in the middle fo test code.
I tried to fix the issue, however, Case case[] ={}; is invalid in the contexts, so... not sure if there still value to raise issue

@jamesbeyond
Copy link
Contributor Author

@cmonr, your suggestion been addressed. please review again

@cmonr cmonr requested a review from a team August 24, 2018 03:23
@0xc0170
Copy link
Contributor

0xc0170 commented Aug 24, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Aug 24, 2018

Build : SUCCESS

Build number : 2896
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7805/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Aug 24, 2018

@mbed-ci
Copy link

mbed-ci commented Aug 25, 2018

@jamesbeyond
Copy link
Contributor Author

Neither test tests-events-timing nor targets NUCLEO_F401RE got changed in the PR. Verified on my local box with NUCLEO_F401RE, test passed. should be CI target unreliable failure. please re-trigger the tests @0xc0170 @cmonr

@cmonr
Copy link
Contributor

cmonr commented Aug 25, 2018

We've been having F401 issues in CI.
/morph test

@mbed-ci
Copy link

mbed-ci commented Aug 25, 2018

@jamesbeyond
Copy link
Contributor Author

em..... this time GCC passed, IAR failed on the same test, same F401.

@cmonr
Copy link
Contributor

cmonr commented Aug 25, 2018

@jamesbeyond The 401's have now been power cycled...

/morph test

@mbed-ci
Copy link

mbed-ci commented Aug 26, 2018

@0xc0170 0xc0170 merged commit a24cecf into ARMmbed:master Aug 27, 2018
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.

5 participants