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

Cellular Unittests refactored to GoogleTest framework #7944

Merged
merged 4 commits into from Sep 18, 2018

Conversation

Projects
None yet
7 participants
@AnttiKauppila
Contributor

AnttiKauppila commented Aug 31, 2018

Cellular unittests ported to use GoogleTest framework

  • Also deleted old unittests from features/cellular

This can be targeted to 5.10.0-rc2.

Pull request type

[ ] Fix
[X] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change
@AnttiKauppila

This comment has been minimized.

Contributor

AnttiKauppila commented Aug 31, 2018

@AnttiKauppila AnttiKauppila changed the title from Unittests to Cellular Unittests refactored to GoogleTest framework Aug 31, 2018

@0xc0170 0xc0170 requested a review from ARMmbed/mbed-os-wan Aug 31, 2018

@jarvte

jarvte approved these changes Sep 3, 2018

@AnttiKauppila AnttiKauppila force-pushed the AnttiKauppila:unittests branch 2 times, most recently from e27313b to ac75018 Sep 5, 2018

@mbed-ci

This comment has been minimized.

mbed-ci commented Sep 9, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@AnttiKauppila

This comment has been minimized.

Contributor

AnttiKauppila commented Sep 10, 2018

Multi ticker test timed out. Has nothing to do with these unit tests I am updating

@AnttiKauppila

This comment has been minimized.

Contributor

AnttiKauppila commented Sep 10, 2018

@0xc0170 Can you retrigger only that one build (or merge this)

@AnttiKauppila AnttiKauppila force-pushed the AnttiKauppila:unittests branch from ac75018 to 99c83f5 Sep 10, 2018

@AnttiKauppila

This comment has been minimized.

Contributor

AnttiKauppila commented Sep 11, 2018

/morph build

@adbridge

This comment has been minimized.

Contributor

adbridge commented Sep 11, 2018

@AnttiKauppila Please DO NOT trigger ci jobs yourself! Especially not during a release campaign. This looks like new functionality and we do not take new functionality into RC releases! @ChiefBureaucraticOfficer please comment

@cmonr

This comment has been minimized.

Contributor

cmonr commented Sep 11, 2018

@AnttiKauppila PLease refrain from starting the builds. @0xc0170 is out this week. If you need to get a hold of the other maintainers, please use @ARMmbed/mbed-os-maintainers.

I've stopped the build since we're prioritizing the 5.10.0-rc2 build.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Sep 11, 2018

Marking for the first patch release for now, since RCs PRs should only be for fixing things found durring OOB.

@AnttiKauppila

This comment has been minimized.

Contributor

AnttiKauppila commented Sep 11, 2018

Right. I would have liked this to land also in rc2, but I can wait for patch release as well.
This contains mostly only unit test related changes. (3 minor issues fixed which were found during testing).
Is it so that before rc2, nothing happens in CI? I have quite many PRs to be made and probably PRs start stacking also with other people. So it might be getting worse all the time

@cmonr

This comment has been minimized.

Contributor

cmonr commented Sep 11, 2018

This contains mostly only unit test related changes. (3 minor issues fixed which were found during testing).

Good to know. If those three fixed issues were spun out into their own PR, I think we could take them for RC3. Otherwise, our policy is to take in test improvements and release them as often as possible.

Is it so that before rc2, nothing happens in CI? I have quite many PRs to be made and probably PRs start stacking also with other people. So it might be getting worse all the time

It's definitely more of a problem this release than most, since a couple of weeks ago we were averaging around 70-80 open PRs...
Every once in a while, if we need to push something through CI that is of dritical importance, we'll stop pending jobs and restart them when appropriate. It's not as bad as it sounds because at most, one PR is active in any given stage (Build/Export/Test) (actually, I think we run two Export PRs in parallel), and the rest are in a queue waiting to start.

Definitely hoping we can implement speed improvements without negatively affecting stability after the release.

@AnttiKauppila AnttiKauppila force-pushed the AnttiKauppila:unittests branch from 99c83f5 to abfbd9c Sep 14, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Sep 17, 2018

Still needs a rebase, there's a conflict

@0xc0170 0xc0170 added needs: work and removed needs: review labels Sep 17, 2018

@AnttiKauppila AnttiKauppila force-pushed the AnttiKauppila:unittests branch from abfbd9c to 2b7a087 Sep 17, 2018

@AnttiKauppila

This comment has been minimized.

Contributor

AnttiKauppila commented Sep 17, 2018

@0xc0170 Rebased

@cmonr cmonr added needs: CI and removed needs: work labels Sep 17, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Sep 18, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Sep 18, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Sep 18, 2018

@cmonr cmonr merged commit f00c564 into ARMmbed:master Sep 18, 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: successful
Details
travis-ci/astyle Passed, 604 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 9958 cycles (+799 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

@cmonr cmonr removed the ready for merge label Sep 18, 2018

@AnttiKauppila AnttiKauppila deleted the AnttiKauppila:unittests branch Sep 19, 2018

@SierpG

This comment has been minimized.

SierpG commented Sep 20, 2018

I got the latest version of the mbed master branch and I called in a MinGW environment in the UNITTESTS directory "python mbed_unittest.py".
However I got the following error: libat_cellular_device_unit.MbedOS.a(AT_CellularDevice.cpp.obj):AT_CellularDevice.cpp:(.text+0x18): undefined reference to `mbed::CellularDevice::CellularDevice()'

After changing the file
....\mbed-os-master\UNITTESTS\features\cellular\framework\AT\at_cellulardevice\unittest.cmake to

# Source files
set(unittest-sources
stubs/randLIB_stub.c
../features/cellular/framework/AT/AT_CellularDevice.cpp
../features/cellular/framework/device/CellularDevice.cpp # <==This line was missing
)

compiling and linking of the unit tests went ok.

@AnttiKauppila

This comment has been minimized.

Contributor

AnttiKauppila commented Sep 20, 2018

Fix is already waiting to be merged:
#8176

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