-
Notifications
You must be signed in to change notification settings - Fork 212
Relax timing on ClientWrapperTest #1716
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a bug-fix, please can this target 0.10 branch rather than master.
Some other comments too.
Yikes, git-splosion. Let me try that again... |
Should be better now... every time I think I know what |
I've always given up and just cherry-picked in the past! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A minor style change, a style suggestion and a query. Other than that, pending CI this looks good to merge thanks!
Co-authored-by: Peter Newman <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks
Looks like it's perhaps still not relaxed enough on Arm on a VM (sometimes at least) https://travis-ci.org/github/OpenLightingProject/ola/jobs/759708456 : Traceback (most recent call last): File "../../python/ola/ClientWrapperTest.py", line 201, in testEventLoop
AssertionError: datetime.timedelta(0, 0, 5709) != datetime.timedelta(0, 0, 5000) within datetime.timedelta(0, 0, 500) delta |
Fixes #1714.
I allowed up to 500 microseconds of drift in the checks, this way we still catch anything that's wildly off but allow for some minor variance in Python's execution.