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 GTest support #381

Merged
merged 2 commits into from
Mar 24, 2017
Merged

Add GTest support #381

merged 2 commits into from
Mar 24, 2017

Conversation

sethfowler
Copy link
Contributor

We really need to start writing more unit tests. One reason that keeps getting put off is that we've agreed that we want to use GTest, but nobody has had the time to actually integrate GTest into the build system. I took yesterday's hackathon as an opportunity to do so.

e925048 adds GTest as a submodule.

4bba2ac adds GTest support to the build system. Tests can be added from any Makefile by adding them to gtest_SOURCES. All GTests will be linked into a single GTest executable, gtestp4c. gtestp4c will be automatically run during make check, or you can run it directly if you want to use GTest's nice features.

To kick things off, I translated an existing unit test (opeq_test) into a GTest.

@sethfowler sethfowler self-assigned this Mar 22, 2017
@sethfowler
Copy link
Contributor Author

I should note that I've confirmed that a failing GTest triggers a make check failure and that the log shows up in test-suite.log. I've also tested adding additional GTests from extensions.

@mihaibudiu
Copy link
Contributor

We have maybe 8 unit tests, but we have a few hundred compiler tests.
Will the latter take advantage of gtest in any way?

@sethfowler
Copy link
Contributor Author

@mbudiu-vmw the answer to that question right now is "maybe", which isn't very satisfying, I know. =)

We've talked about whether we could gain some benefits from using GTest to drive the compiler tests. Making that change could let us get rid of some hacks, but it's probably not worth putting a lot of work into, so I think the current consensus is "we'll do it if it's very simple". We don't know yet whether it's simple, though, because nobody has actually tried to do it.

It's true that we only have 8 unit tests, by the way, but I'm hoping to encourage people to write more. =)

@sethfowler
Copy link
Contributor Author

The test failed due to cpplint. Doh. I'll push a fix.

@ChrisDodd
Copy link
Contributor

So what's the upside of this? It doesn't look like gtest provides any extra functionality we don't already have, just requires writing different boilerplate in the test, and doesn't really save or improve anything.

@antoninbas
Copy link
Member

@ChrisDodd many pre-defined test assertions, value-parametrized tests, type-parametrized tests, exception support,... I would say all of these things are pretty useful if you guys plan on writing unit tests more systematically.

@sethfowler
Copy link
Contributor Author

Thanks! I'll rebase this and get it landed later today.

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.

5 participants