Code review comment for lp:~aacid/unity/do_not_reuse_menus_on_order_change

Revision history for this message
Marco Trevisan (TreviƱo) (3v1n0) wrote :

331 + DBusIndicatorsTest* dbus_indicators;

Would be nice to have DBusIndicators::Ptr instead, so you don't need to worry about destruction (unless it must be done before unreffing the loop)

336 +TEST_F(TestDBusIndicators, TestConstruction)
337 +{
338 + SetUp();

357 + TearDown();
358 +}

No need to do this... GTest will do this automatically, so forget about setting-up and tearing-down once you've defined these testing::Test functions.

341 + auto timeout_check = [&] () -> bool
342 + {
343 + nChecks++;
344 + bool quit_loop = dbus_indicators->IsConnected() || nChecks > 99;
345 + if (quit_loop)
346 + g_main_loop_quit(loop_);
347 + return !quit_loop;
348 + };
349
350 + nChecks = 0;
351 + timeout.reset(new glib::Timeout(100, timeout_check));
352 +
353 + g_main_loop_run(loop_);

No need to repeat this for every test...

Just do it in the SetUp function, and probably instead of quitting the loop is enough to add an ASSERT there.

« Back to merge proposal