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

Fix SocketAddress unittests #8086

Merged
merged 1 commit into from Sep 14, 2018

Conversation

Projects
None yet
8 participants
@SeppoTakalo
Contributor

SeppoTakalo commented Sep 11, 2018

Description

Add ip4tos() and stoip4() into SocketAddress unittests.
NOTE: Probably should have been stubbed but this way we can also
test these helper functions.

#6293 Added usage of these helper functions, but by that time unit tests were not in place.

CC: @OPpuolitaival @kjbracey-arm

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change
Fix SocketAddress unittests
Add ip4tos() and stoip4() into unittests.
NOTE: Probably should have been stubbed but this way we can also
test these helper functions.
@OPpuolitaival

This comment has been minimized.

Contributor

OPpuolitaival commented Sep 12, 2018

@lorjala please review

@lorjala

This comment has been minimized.

Contributor

lorjala commented Sep 12, 2018

Looks good, but is identical to this #8018

Which one we choose? This one implements actual headers.

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Sep 12, 2018

Oh...
I would actually pull in the actual code into the test.
But I know some people would prefer dummy stubs that do nothing.

I prefer to pull in library code into unittests, so that the test can actually do something useful.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Sep 12, 2018

@SeppoTakalo @OPpuolitaival @kjbracey-arm @lorjala

If I understand, are we going to prefer this PR over #8018?

@lorjala

This comment has been minimized.

Contributor

lorjala commented Sep 12, 2018

@cmonr we're going with this.

@cmonr cmonr added needs: CI and removed needs: review labels Sep 12, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Sep 12, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Sep 12, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@adbridge

This comment has been minimized.

Contributor

adbridge commented Sep 13, 2018

/morph test

1 similar comment
@cmonr

This comment has been minimized.

Contributor

cmonr commented Sep 13, 2018

/morph test

@cmonr

This comment has been minimized.

Contributor

cmonr commented Sep 13, 2018

Trying that again...
/morph test

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Sep 13, 2018

Stopped job to help root cause network issue.

@adbridge

This comment has been minimized.

Contributor

adbridge commented Sep 14, 2018

Kicking off again to see if we have more luck this morning!
/morph test

@mbed-ci

This comment has been minimized.

@adbridge adbridge added ready for merge and removed needs: CI labels Sep 14, 2018

@cmonr cmonr merged commit 48d212f into ARMmbed:master Sep 14, 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, 598 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 10281 cycles (-260 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment