Skip to content

Initial port of UnitTest++ to VxWorks 5.5. #121

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

sulemankm
Copy link

@sulemankm sulemankm commented Jul 26, 2016

UnitTest++ ported to VxWorks 5.5. All tests pass on VxWorks 5.5.

….cpp compiles but its tests don't build for VxWorks
@sulemankm sulemankm changed the title Initial port of UnitTest++ to VxWorks 5.5. NOTE: DefferedTestReporter… Initial port of UnitTest++ to VxWorks 5.5. Jul 26, 2016
@pjohnmeyer
Copy link
Member

Ah, VxWorks.

Thanks for the PR. I'm happy to incorporate changes to support VxWorks; worked with it once myself. I'm going to take a closer look sometime soon, but my first question is:

Why do we need all of the VxWorks toolchain-specific project files? Is there no combination of CMake options that will work? For example, could you incorporate a custom toolchain file into your process? Or perhaps just use the Unix Makefile generator with the compiler and linker options set correctly?


Timer::Timer()
{
m_startTime.tv_sec = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Prefer initializing members in the constructor initializer list.

Copy link
Author

Choose a reason for hiding this comment

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

Yes that can be done but I just implemented it in the same way as done in the Posix version of TimeHelpers.cpp.

@pjohnmeyer
Copy link
Member

Overall my main concern with this stands; the VxWorks toolchain-specific project files ideally don't belong here. I'd be happy to discuss elsewhere how we might manage this; perhaps we can spin up a new repository within the organization that @sulemankm could maintain?

Even if we solve that problem, there's not currently a way to 'inject' the TimeHelper class into the build.

@sulemankm
Copy link
Author

sulemankm commented Aug 18, 2016

Actually tornado generates a Makefile which can used to build it for Vxworks. However, Tornado/VxWorks toolchain installation is required for building the framework and test anyway. So as installation of Tornado/VxWorks toolchain is a must, then Tornado project fils would come in handy for building the UnitTest++ framework and tests.

Pardon my ignorance but, I'm not really sure what you mean by not being able to "inject" the TimeHelper class into the build.

@pjohnmeyer
Copy link
Member

By "inject" I was hinting at dependency injection; right now the correct TimeHelper is selected at compile time, rather than by a factory or anything like that. If we were to move all of the VxWorks toolchain-specific stuff out into a separate project as I proposed, we would still have to include the VxWorks-specific time helper in the core UnitTest++ build.

@sulemankm
Copy link
Author

Ok, I got your point about injecting dependencies. About the build solution, I think pre-compile-time selection of the appropriate header file is probably the easiest solution.

@pjohnmeyer
Copy link
Member

@sulemankm sorry I left this unattended for so long.

I'm interested in making some of these changes but not all of them. If we can get rid of the VxWorks project files, and figure out how to address the .at issue -- I'm thinking by having GetResults return a wrapper object for VxWorks -- then I'm game. If you're still interested in getting this pull folded in, can you make those changes? Or, add me as a collaborator to this pull request and I'll take a stab at it -- although you'll have to check the compiling for me.

@sulemankm
Copy link
Author

How about GetResults return an object with an overloaded [] operator which internally calls the .at() method for vxWorks. Just a wild idea. I haven't tried it so I'm not sure if it is even feasible. Just a wild idea!

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