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

Projects
None yet
5 participants
@jamesbeyond
Contributor

jamesbeyond commented Aug 16, 2018

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

@cmonr cmonr requested review from c1728p9 and cmonr Aug 16, 2018

@cmonr cmonr added the needs: review label Aug 16, 2018

jamesbeyond added some commits Aug 14, 2018

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.

@jamesbeyond jamesbeyond force-pushed the jamesbeyond:fm_test branch from 12c02d6 to 485fe79 Aug 17, 2018

@jamesbeyond

This comment has been minimized.

Contributor

jamesbeyond commented Aug 17, 2018

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

This comment has been minimized.

Member

0xc0170 commented Aug 20, 2018

This PR have a dependency on #7706

Merged. This is ready for review!

@cmonr cmonr changed the title from Fix the Greentea tests for Mbed OS code coverage on Fast Models to 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)

This comment has been minimized.

@cmonr

cmonr Aug 21, 2018

Contributor

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.

This comment has been minimized.

@jamesbeyond

jamesbeyond Aug 21, 2018

Contributor

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

This comment has been minimized.

@cmonr

cmonr Aug 23, 2018

Contributor

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

This comment has been minimized.

Contributor

jamesbeyond commented Aug 22, 2018

@cmonr please have another look

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

This comment has been minimized.

@0xc0170

0xc0170 Aug 23, 2018

Member

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

This comment has been minimized.

@jamesbeyond

jamesbeyond Aug 23, 2018

Contributor

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

This comment has been minimized.

Contributor

jamesbeyond commented Aug 23, 2018

@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

This comment has been minimized.

Contributor

jamesbeyond commented Aug 23, 2018

@cmonr, your suggestion been addressed. please review again

@cmonr cmonr requested a review from ARMmbed/mbed-os-test Aug 24, 2018

@cmonr

cmonr approved these changes Aug 24, 2018

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Aug 24, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Aug 24, 2018

/morph build

@mbed-ci

This comment has been minimized.

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

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@jamesbeyond

This comment has been minimized.

Contributor

jamesbeyond commented Aug 25, 2018

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

This comment has been minimized.

Contributor

cmonr commented Aug 25, 2018

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

@mbed-ci

This comment has been minimized.

@jamesbeyond

This comment has been minimized.

Contributor

jamesbeyond commented Aug 25, 2018

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

@cmonr

This comment has been minimized.

Contributor

cmonr commented Aug 25, 2018

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

/morph test

@mbed-ci

This comment has been minimized.

@cmonr cmonr added ready for merge and removed needs: CI labels Aug 26, 2018

@0xc0170 0xc0170 merged commit a24cecf into ARMmbed:master Aug 27, 2018

15 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed , RTOS ROM(+0.0%) RAM(+0.0%)
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/cloud_client_smoke_test Test job was successful
Details
travis-ci/astyle Passed, 585 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 9213 cycles (-882 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8372B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details

pan- pushed a commit to pan-/mbed that referenced this pull request Aug 29, 2018

Merge pull request ARMmbed#7805 from jamesbeyond/fm_test
Skip Greentea tests for Mbed OS code coverage on Fast Models
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment