Skip to content

Provide fixes for Timer and LowPowerTimer tests. #5403

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Nov 16, 2017
Merged

Provide fixes for Timer and LowPowerTimer tests. #5403

merged 3 commits into from
Nov 16, 2017

Conversation

mprse
Copy link
Contributor

@mprse mprse commented Oct 30, 2017 •
edited
Loading

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 Issue with the expected tolerance in the Low Power Timer test #5468. Increased DELTA value.

Status

READY

Migrations

NO

@mprse mprse force-pushed the timer_test_delta_fix branch from ac38154 to 825584d Compare October 30, 2017 15:00
@0xc0170 0xc0170 requested review from c1728p9 and a user November 2, 2017 09:56
@mprse mprse mentioned this pull request Nov 3, 2017
Merged
@0xc0170
Copy link
Contributor

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.

@mprse mprse force-pushed the timer_test_delta_fix branch from 825584d to 992a43f Compare November 6, 2017 14:25
@mprse
Copy link
Contributor Author

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 3 commits November 13, 2017 10:12
@mprse
Provide fix for Timer and LowPowerTimer tests (time accumulation).
d49899a 
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.
@mprse
Move Timer test, modify ASERT macros, fix comments.
d962f67 
Move Timer test to TESTS/mbed_drivers/ directory which is more suitable.
Fix few comments which are incorrect.
Use more relevant ASERT macros.
@mprse
Provide fix for Issue ARMmbed#5468.
78e1362 
Issue: ARMmbed#5468
Increased DELTA value for Timer and Low Power Timer tests.
@mprse mprse force-pushed the timer_test_delta_fix branch from 992a43f to 78e1362 Compare November 13, 2017 10:06
@mprse
Copy link
Contributor Author

mprse commented Nov 13, 2017

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

@mprse mprse mentioned this pull request Nov 13, 2017
Closed
@mprse mprse changed the title Provide fix for Timer and LowPowerTimer tests. Provide fixes for Timer and LowPowerTimer tests. Nov 13, 2017
@0xc0170
Copy link
Contributor

0xc0170 commented Nov 14, 2017

/morph build

@mbed-ci
Copy link

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
Copy link
Contributor

0xc0170 commented Nov 14, 2017

/morph test

@mbed-ci
Copy link

mbed-ci commented Nov 14, 2017

Exporter Build : SUCCESS

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

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 15, 2017

/morph test

@mbed-ci
Copy link

mbed-ci commented Nov 15, 2017

Test : SUCCESS

Build number : 324
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5403/324

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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@c1728p9 c1728p9 Awaiting requested review from c1728p9

Assignees
No one assigned
Labels
release-version: 5.6.5
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mprse @0xc0170 @mbed-ci @theotherjimmy @adbridge