Did you know... gtest?

60 views
Skip to first unread message

Paweł Hajdan, Jr.

unread,
Sep 15, 2011, 8:13:04 PM9/15/11
to chromium-dev
Are you writing gtest (googletest) based tests? Read on. During my drive-by reviews I discovered a lot of cases where test code could be improved or simplified if people knew gtest better.

I'd like to strongly encourage everyone to read http://code.google.com/p/googletest/wiki/Primer and at least skim through http://code.google.com/p/googletest/wiki/AdvancedGuide, even if you think you know gtest.

Let me also paste one of the most important points from the documentation:

For each test defined with TEST_F(), Google Test will:
  1. Create a fresh test fixture at runtime
  2. Immediately initialize it via SetUp() ,
  3. Run the test
  4. Clean up by calling TearDown()
  5. Delete the test fixture. Note that different tests in the same test case have different test fixture objects, and Google Test always deletes a test fixture before it creates the next one. Google Test does not reuse the same test fixture for multiple tests. Any changes one test makes to the fixture do not affect other tests.
Please note that it means you can assume in SetUp that your test fixture class instance is freshly constructed. You don't need to undo anything in SetUp. Similarly, the test fixture class members will be destroyed between tests. In most cases there is no need to use scoped_ptrs and manually manage things in SetUp and TearDown.

There is still some trickyness though: it is quite common to have to run MessageLoop::current()->RunAllPending() in TearDown. Often just destroying the message loop may drop some tasks without running them.

Other common thing are temporary files or directories: please just use ScopedTempDir (there are lots of examples in code, most of them are good).

I'd like to thank David Grogan for effectively making me write this post, and thank you for reading it.

Peter Kasting

unread,
Sep 15, 2011, 8:16:17 PM9/15/11
to phajd...@chromium.org, chromium-dev
On Thu, Sep 15, 2011 at 5:13 PM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:
it means you can assume in SetUp that your test fixture class instance is freshly constructed. You don't need to undo anything in SetUp. Similarly, the test fixture class members will be destroyed between tests. In most cases there is no need to use scoped_ptrs and manually manage things in SetUp and TearDown.

Wow.  I assumed otherwise because so much of our code does these sorts of things.  Thanks for mailing this.  I wish we had an easy way to quickly find and fix all these so people in the future won't be misled.

PK

Denis Lagno

unread,
Sep 16, 2011, 7:33:20 AM9/16/11
to phajd...@chromium.org, chromium-dev
On Fri, Sep 16, 2011 at 4:13 AM, Paweł Hajdan, Jr.
<phajd...@chromium.org> wrote:
> In most cases there is no need to use scoped_ptrs and manually manage
> things in SetUp and TearDown.

sorry, could you clarify on scoped_ptrs?

Mattias Nissler

unread,
Sep 16, 2011, 7:42:43 AM9/16/11
to dil...@chromium.org, phajd...@chromium.org, chromium-dev
What he refers to is that you can just use regular non-pointer class members for many things. There are a lot of tests that instead have scoped_ptr class members that get initialized in SetUp().
 

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
   http://groups.google.com/a/chromium.org/group/chromium-dev

Paweł Hajdan, Jr.

unread,
Sep 16, 2011, 12:16:30 PM9/16/11
to Mattias Nissler, dil...@chromium.org, chromium-dev
Exactly. So you can just do:

class SomeTest : public testing::Test {
 protected:
  TestedClass tested_class_;
}

instead of having a scoped_ptr dynamically initialized in SetUp and explicitly reseted in TearDown. The code above is just much simpler.

A possible exception would be for example if you need to MessageLoop::current()->RunAllPending() after destroying one object and before another is destroyed (e.g. the message loop itself). If you need scoped_ptrs in test fixtures, they're fine. But in many cases they're really not needed.
Reply all
Reply to author
Forward
0 new messages