Skip to content
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

Splitup the Test Macro file, alternative #953

Merged
merged 5 commits into from
May 3, 2016
Merged

Conversation

basvodde
Copy link
Member

@basvodde basvodde commented May 2, 2016

Alternative split by splitting out String functions

@arstrube
Copy link
Contributor

arstrube commented May 2, 2016

I have no objections to this solution, but could you please put the new files into sources.mk for Dos and see that the Dos build is indeed fixed. I am not sure about this, as the long long functions are prone to generate a lot of assembler code and I have no idea whether the string funcitons will outweigh them.

@arstrube
Copy link
Contributor

arstrube commented May 2, 2016

Needless to say that TestTestingFixture is cleaner this way :-)

@basvodde
Copy link
Member Author

basvodde commented May 2, 2016

It still needs to be cleaned up a bit though...

@arstrube
Copy link
Contributor

arstrube commented May 2, 2016

I'm kind of fond of my "hack" in the header though. I wouldn't have thought I could make the static functiosn work without a source file :P

@basvodde
Copy link
Member Author

basvodde commented May 2, 2016

I'd prefer moving everything to the source file anyway. It was already something that should have happened.

@arstrube
Copy link
Contributor

arstrube commented May 2, 2016

Can't really dispute that :D.

Are you going to make the changes needed to platforms/dos/sources.mk? Or how should we approach that? Should I submit a PR against basvodde/master?

@basvodde
Copy link
Member Author

basvodde commented May 2, 2016

Yes, will gradually :) Let me see the failure reasons.

@arstrube
Copy link
Contributor

arstrube commented May 2, 2016

For starters and obvious reasons, you need src/CppUTest/TestTestingFixture.cpp in the dos buid....

@arstrube
Copy link
Contributor

arstrube commented May 2, 2016

Same goes for Cmake builds.

Interesting failures for Autotools, and also interesting that Cygwin succeded :-/

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 99.873% when pulling 167c7f3 on basvodde:master into 9c9f347 on cpputest:master.

@@ -14,7 +14,7 @@ set(CppUTest_src
MemoryLeakDetector.cpp
TestFilter.cpp
TestPlugin.cpp
TestTestingPlugin.cpp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/me an idiot not paying attention :)

@arstrube
Copy link
Contributor

arstrube commented May 2, 2016

The FAIL_LOCATION lines need to be commented with // LCOV_EXCL_LINE to make Coveralls happy

@basvodde
Copy link
Member Author

basvodde commented May 3, 2016

I'll merge this, the Vistual Studio files need fixing still though :(

@basvodde basvodde merged commit d5a81ef into cpputest:master May 3, 2016
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.

None yet

3 participants