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

Add support for GTest based UnitTests #119

Merged
merged 3 commits into from Mar 3, 2014
Merged

Conversation

delroth
Copy link
Member

@delroth delroth commented Mar 2, 2014

No support for MSVC at the moment. Fairly basic but should be enough to start adding tests for a few things.

include(FindGTest OPTIONAL)
if(GTEST_FOUND)
include_directories(${GTEST_INCLUDE_DIRS})
enable_testing()

This comment was marked as off-topic.

@Sonicadvance1
Copy link
Contributor

LGTM

@lioncash
Copy link
Member

lioncash commented Mar 2, 2014

Looks good. I'm alright with this.

@comex
Copy link
Contributor

comex commented Mar 2, 2014

@delroth
Copy link
Member Author

delroth commented Mar 2, 2014

@comex I'm planning to at least try and unittest a few modules that are not strongly coupled to the rest of Dolphin. For the rest, I'll be waiting for @degasus' null video backend to implement some larger scale tests (+ reusing the fifoci framework for some GFX tests).

@BhaaLseN
Copy link
Member

BhaaLseN commented Mar 2, 2014

I'm not terribly happy that you put up to 3 statements on one line (almost missed the EQ in the last test and wanted to complain about it)...but then again, yay, tests! \o/

@delroth
Copy link
Member Author

delroth commented Mar 2, 2014

.Yeah, I'm not a fan of the 3 statements per line either... I found it more readable than the alternative 3 lines though. I could in theory templatize that, but I'd prefer to keep tests simple.

I could fix it if you suggest a better way.

@neobrain expressed concerns about having tests built by default. At this point I think it is not a huge problem and we can revisit this decision later if tests start taking a long time to build. Ideally we would build tests only when running "make test", but I can't find a good way to do that (might have to ask on a CMake ML). Opinion on that?

@delroth
Copy link
Member Author

delroth commented Mar 2, 2014

Now tests are not built by default and are only built when trying to run the testsuite with "make unittests". Unfortunately, because cmake is a dumb tool, "make test" is and will be broken - there is no way around that at the moment (according to CMake devs).

@Parlane
Copy link
Member

Parlane commented Mar 3, 2014

LGTM

Parlane added a commit that referenced this pull request Mar 3, 2014
Add support for GTest based UnitTests
@Parlane Parlane merged commit 6176424 into dolphin-emu:master Mar 3, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
7 participants