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

Unit testing framework #7819

Merged
merged 2 commits into from Aug 29, 2018

Conversation

@lorjala
Contributor

lorjala commented Aug 17, 2018

Description

Unit testing framework for writing, building and running Mbed OS unit tests for developers and contributors.

  • Separate test binaries
  • No HW and SW dependencies for testable code
  • Linux, Mac OS and Windows

MBEDOSTEST-3

Pull request type

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

@lorjala lorjala force-pushed the lorjala:unittests branch 2 times, most recently from eec3b46 to 5b22b87 Aug 17, 2018

@lorjala lorjala force-pushed the lorjala:unittests branch 2 times, most recently from f9419b2 to 41c5cff Aug 17, 2018

@cmonr cmonr requested review from neilwjackson, ChiefBureaucraticOfficer, sg- and ARMmbed/mbed-os-maintainers Aug 17, 2018

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

static uint32_t count = 0;
// Test stubs
osStatus_t osSemaphoreAcquire (osSemaphoreId_t semaphore_id, uint32_t timeout) { return retval; }

This comment has been minimized.

@0xc0170

0xc0170 Aug 20, 2018

Member

examples should follow our coding style (please run astyle on this code and update the example)

{
char *n = 0;
EXPECT_EQ(iface->get_mac_address(), n);
}

This comment has been minimized.

@0xc0170

0xc0170 Aug 20, 2018

Member

No newline in t hese files? Leave one please

### Testing with Python script:
```
./UNITTESTS/mbed_unittest.py test

This comment has been minimized.

@0xc0170

0xc0170 Aug 20, 2018

Member

Is this how we want to introduce this tools ? Shouldn't this be mbed unittest rather? and this scripts to be moved into tools/ folder

@@ -0,0 +1,13 @@
#include "events/mbed_shared_queues.h"

This comment has been minimized.

@0xc0170

0xc0170 Aug 20, 2018

Member

Add license headers to the new files, like this one

@@ -0,0 +1,24 @@
#include "rtos/EventFlags.h"

This comment has been minimized.

@0xc0170

0xc0170 Aug 20, 2018

Member

ATHandler_stub - has suffix, but some of the stubs do not have, like this one. Is there a naming convention?

This comment has been minimized.

@lorjala

lorjala Aug 21, 2018

Contributor

Few example unit tests are from features/cellular/UNITTESTS without major changes to demonstrate how to convert existing unit tests to use this framework. So ATHandler_stub for example is from these existing tests which uses that naming convention. These unit tests introduced in this PR are not final in any form including naming. Their purpose is only to showcase the framework before proper unit tests are written/converted.

@0xc0170 0xc0170 added needs: work and removed needs: review labels Aug 20, 2018

@lorjala lorjala referenced this pull request Aug 20, 2018

Merged

Support for unit testing #734

### Testing with Mbed CLI
```
mbed unittest test

This comment has been minimized.

@deepikabhavnani

deepikabhavnani Aug 20, 2018

Contributor

Please describe what will be the behavior of new code when executing mbed compile. All C/CPP files inside unittest folder will not be build or will be skipped during linking step?

This comment has been minimized.

@0xc0170

0xc0170 Aug 20, 2018

Member

I am now reviewing Icetea addition. So back to this mbed unittest comment I had earlier today.
Icetea has it as mbed test -m <target> -t <toolchain> --icetea . This should follow it ? as mbed test -m <target> -t <toolchain> --unittest or just this unittest

Just looking for alignment (icetea / unittest/ app test) - if there can be any. Adding my comment here to this line (hijacking @deepikabhavnani question that is however related)

@OPpuolitaival Please review

This comment has been minimized.

@SeppoTakalo

SeppoTakalo Aug 20, 2018

Contributor

No, the unittesting is basically target and toolchain agnostic.
It runs the one provided by the host OS.

So it is so different from what mbed test is targeting, that it requires its own command.

This comment has been minimized.

@OPpuolitaival

OPpuolitaival Aug 20, 2018

Contributor

@SeppoTakalo is right, we need separated command because:
"mbed test" - testing code which runs on embedded device and therefore need a target and toolchain
"mbed unittest" - testing code units in host computer using documented compilers

This comment has been minimized.

@0xc0170

0xc0170 Aug 21, 2018

Member

+1, own command

### Testing with Mbed CLI
```
mbed unittest test

This comment has been minimized.

@OPpuolitaival

OPpuolitaival Aug 20, 2018

Contributor

@SeppoTakalo is right, we need separated command because:
"mbed test" - testing code which runs on embedded device and therefore need a target and toolchain
"mbed unittest" - testing code units in host computer using documented compilers

Traditional software testing is defined into three main levels: unit testing, integration testing and system testing. These levels are often pictured as a pyramid to indicate the amount of testing per level.
```
^ Testing level

This comment has been minimized.

@OPpuolitaival

OPpuolitaival Aug 20, 2018

Contributor

@SeppoTakalo should we move this picture in testing introduction in Handbook?

* GCC 6 or later
* MinGW-W64 GCC-6.4.0 or MinGW-W64 GCC-7.3.0 (Windows)
* CMake 3.0+ installed.
* Python and pip installed.

This comment has been minimized.

@OPpuolitaival

OPpuolitaival Aug 20, 2018

Contributor

Which python versions we support? Need to define. And maybe pip also. Something like python 2.7.x or >3.5

* MinGW-W64 GCC-6.4.0 or MinGW-W64 GCC-7.3.0 (Windows)
* CMake 3.0+ installed.
* Python and pip installed.
* gcovr and optionally virtualenv pip modules installed.

This comment has been minimized.

@OPpuolitaival

OPpuolitaival Aug 20, 2018

Contributor

gcovr version definition?

mbed unittest test
```
##### Parameters to mbed unittest test

This comment has been minimized.

@OPpuolitaival

OPpuolitaival Aug 20, 2018

Contributor

I don't feel good to list these here because these are coming from mbed-cli and mbed-cli works with any version of Mbed OS. Then we don't have guarantee that these are like this forever.

mbed unittest new
```
##### Parameters to mbed unittest new

This comment has been minimized.

@OPpuolitaival

OPpuolitaival Aug 20, 2018

Contributor

Don't list parameters here because there is no guarantee. Just command and why it exist is good enough

@lorjala lorjala force-pushed the lorjala:unittests branch 2 times, most recently from 2f6a9bd to 714d6b3 Aug 21, 2018

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Aug 21, 2018

Is it a bit too early to describe mbed unittest command here?
First we need this in,
then we can add the command into mbed-cli.

Also, don't remove anything from the README.md, once ready, we just submit part of it into Mbed OS handbook.

@lorjala

This comment has been minimized.

Contributor

lorjala commented Aug 21, 2018

Referencing Mbed CLI here should be ok, because it is mentioned elsewhere in the repo as well(case greentea), and it would not take long to get new mbed-cli release out (PR is ongoing).

@lorjala lorjala force-pushed the lorjala:unittests branch from 714d6b3 to ff7bfdb Aug 21, 2018

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Aug 21, 2018

OK.
Thanks.
PR already here ARMmbed/mbed-cli#734

@lorjala lorjala force-pushed the lorjala:unittests branch from ff7bfdb to 501b4c2 Aug 22, 2018

@0xc0170 0xc0170 requested a review from ARMmbed/mbed-os-test Aug 22, 2018

@cmonr cmonr added the risk: G label Aug 23, 2018

@0xc0170

@theotherjimmy Please check the location for the scripts here (in the UNITTESTS folder rather than tools/unittests or similar). If that is fine.

@lorjala lorjala force-pushed the lorjala:unittests branch from 926e6da to 1ae1059 Aug 23, 2018

@lorjala lorjala force-pushed the lorjala:unittests branch from d21a6a6 to 4dde13d Aug 27, 2018

@lorjala

This comment has been minimized.

Contributor

lorjala commented Aug 27, 2018

@0xc0170 @cmonr can you trigger the build?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Aug 27, 2018

@0xc0170 @cmonr can you trigger the build?

I would like to have first all approvals (currently one requested changes) and someone from @ARMmbed/mbed-os-test reviewed

@OPpuolitaival

This comment has been minimized.

Contributor

OPpuolitaival commented Aug 27, 2018

@0xc0170 I accepted this

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Aug 27, 2018

/morph build

@0xc0170 0xc0170 added needs: CI and removed needs: work labels Aug 27, 2018

@mbed-ci

This comment has been minimized.

mbed-ci commented Aug 27, 2018

Build : SUCCESS

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

Triggering tests

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

@cmonr cmonr added risk: G and removed risk: A labels Aug 27, 2018

@mbed-ci

This comment has been minimized.

```
virtualenv pyenv
pyenv\\Scripts\\activate
pip install 'gcovr>=4.1'

This comment has been minimized.

@jamesbeyond

jamesbeyond Aug 27, 2018

Contributor

All looks good, well done! I give a quick try, everything works just fine, apart from this line.
Seems on my windows 10, pip not like the single quotes, but only works with double quotes. please confirm @lorjala

This comment has been minimized.

@lorjala

lorjala Aug 28, 2018

Contributor

Yes, single quotes do not work when using default command prompt, but they do with powershell.

@mbed-ci

This comment has been minimized.

@lorjala

This comment has been minimized.

Contributor

lorjala commented Aug 28, 2018

@0xc0170 @cmonr This can be merged (trigger the build again if necessary).

@cmonr

This comment has been minimized.

Contributor

cmonr commented Aug 28, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Aug 28, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@cmonr cmonr merged commit 7ecf111 into ARMmbed:master Aug 29, 2018

14 checks passed

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, 557 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 10291 cycles (+1152 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

@lorjala lorjala deleted the lorjala:unittests branch Aug 31, 2018

@cmonr cmonr removed the risk: G label Sep 2, 2018

@theotherjimmy theotherjimmy referenced this pull request Sep 4, 2018

Merged

Document Unit Testing #703

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment