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

Provide fixes for Timer and LowPowerTimer tests. #5403

Merged
merged 3 commits into from Nov 16, 2017

Conversation

Projects
None yet
6 participants
@mprse
Member

mprse commented Oct 30, 2017

Description

  • LowPoterTimer test gives sometimes failed result while testing measured time accumulation. The check which verifies if total number of elapsed milliseconds is valid fails. Test assumes that delta value equal to 1 ms is sufficient for all test cases, which is not true since in case where time measurement is performed few times in sequence the measurement error also accumulates and 1 ms might be not enough. To solve this problem delta value for milliseconds tests must be updated.
  • Since Timer test location is invalid it has been moved to more suitable location: TESTS/mbed_drivers/. Additionally fixed invalid comments and used more suitable ASERT macros.
  • Provide fix for Issue #5468. Increased DELTA value.

Status

READY

Migrations

NO

@mprse mprse force-pushed the mprse:timer_test_delta_fix branch Oct 30, 2017

@0xc0170 0xc0170 requested review from c1728p9 and scartmell-arm Nov 2, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Nov 6, 2017

From the commit msg:

Additional changes:
- provide fix also for Timer test.
- move Timer test to TESTS/mbed_drivers/ directory which is more suitable.
- fix minor issues and comments.

The check if number of elapsed mili seconds simetimes fails. Test assumes that

Seems like msg was truncated, is that correct? Why are you squashing these 3 changes? I do not see which one is bugfix and which one are fix comments / minor issues (what are minor issues) ? It might make sense to split this into at least 2 commits.

@0xc0170 0xc0170 added needs: work and removed needs: review labels Nov 6, 2017

@mprse mprse force-pushed the mprse:timer_test_delta_fix branch Nov 6, 2017

@mprse

This comment has been minimized.

Member

mprse commented Nov 6, 2017

Seems like msg was truncated, is that correct? Why are you squashing these 3 changes? I do not see which one is bugfix and which one are fix comments / minor issues (what are minor issues) ? It might make sense to split this into at least 2 commits.

Fixed broken commit message - sorry for that.
Added more detailed information about the changes.
This PR provides fix, but additionally I have also changed few comments and used more suitable ASERT macros. This are these "minor issues" which were catch during the update. Do I need a special commit for this?

mprse added some commits Nov 13, 2017

Provide fix for Timer and LowPowerTimer tests (time accumulation).
LowPoterTimer test gives sometimes failed result while testing measured time accumulation. The check which verifies if total number of elapsed milliseconds is valid fails. Test assumes that delta value equal to 1 ms is sufficient for all test cases, which is not true since in case where time measurement is performed few times in sequence the measurement error also accumulates and 1 ms might be not enough. To solve this problem delta value for milliseconds tests must be updated.
Move Timer test, modify ASERT macros, fix comments.
Move Timer test to TESTS/mbed_drivers/ directory which is more suitable.
Fix few comments which are incorrect.
Use more relevant ASERT macros.
Provide fix for Issue #5468.
Issue: #5468
Increased DELTA value for Timer and Low Power Timer tests.

@mprse mprse force-pushed the mprse:timer_test_delta_fix branch to 78e1362 Nov 13, 2017

@mprse

This comment has been minimized.

Member

mprse commented Nov 13, 2017

Spitted into 2 commits as suggested and added additional commit with fix for issue #5468.

@mprse mprse changed the title from Provide fix for Timer and LowPowerTimer tests. to Provide fixes for Timer and LowPowerTimer tests. Nov 13, 2017

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Nov 14, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Nov 14, 2017

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Nov 14, 2017

Build : SUCCESS

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

Triggering tests

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

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Nov 14, 2017

/morph test

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Nov 15, 2017

/morph test

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 removed the needs: CI label Nov 15, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Nov 15, 2017

@mprse Test passed, this should be good to go!

@0xc0170 0xc0170 merged commit eb5d3ff into ARMmbed:master Nov 16, 2017

6 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment