Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Add "default" empty implementation for Timer and SleepMs()#170

Open
p12tic wants to merge 1 commit into
unittest-cpp:masterunittest-cpp/unittest-cpp:masterfrom
p12tic:timer-add-default-implp12tic/unittest-cpp:timer-add-default-implCopy head branch name to clipboard
Open

Add "default" empty implementation for Timer and SleepMs()#170
p12tic wants to merge 1 commit into
unittest-cpp:masterunittest-cpp/unittest-cpp:masterfrom
p12tic:timer-add-default-implp12tic/unittest-cpp:timer-add-default-implCopy head branch name to clipboard

Conversation

@p12tic

@p12tic p12tic commented Nov 16, 2018

Copy link
Copy Markdown

First, let me thank you for the great library that UnitTest++ is.

This PR upstreams some of the changes we have in a private fork of UnitTest++ at Unity Technologies.

A "default" implementation of Timer and TimeHelpers::SleepMs() has been added which could be used on platforms that either don't support timers or simply are not implemented in UnitTest (which does happen on weird platforms that Unity supports). Additionally, the implementation classes have been renamed to unique names so that accidental ODR clashes are not possible and instead we get a link error. Otherwise, depending on compilation flags, silent stack corruption would result.

@p12tic p12tic force-pushed the timer-add-default-impl branch from f70aeb8 to 452a6c7 Compare November 16, 2018 15:06
@p12tic

p12tic commented Nov 16, 2018

Copy link
Copy Markdown
Author

Note: the CI failure is not limited to this PR and is likely a preexisting problem on the master branch. See e.g. https://travis-ci.org/unittest-cpp/unittest-cpp/builds/322087958

@pjohnmeyer

Copy link
Copy Markdown
Member

@p12tic thanks for the PR. I've been away for a while but I will take a look soon.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.