Port all of inboxpagemodeltest away from mocks
ClosedPublic

Authored by dfaure on Sep 28 2019, 11:18 AM.

Details

Summary
  • Had to extend error handling in akonadifakestorage
  • There is something nasty with the kjobs being triggered for extra

info, from *PageModel::data(). If they keep running past the end
of the test method, they end up calling into a deleted Domain::LiveQueryInput.
This is because JobHandlerInstance is a singleton so it stays around,
the job's signal calls the JobHandlerInstance slot, and from there it's all
about lambdas in deleted objects.
Calling waitForEmptyJobQueue() at the right places avoids this, but it's
still unexpected.

  • I wanted to skip fetching extra info when role == ObjectRole

(we don't need it for that), but this only triggers the above problem
even more, now it's later calls to data() which fetch extra info,
increasing chances of jobs lying around after deletion of everything
else.

Test Plan

Tests pass

Diff Detail

Repository
R4 Zanshin
Branch
2019_09_finish_inboxpagemodeltest
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 17124
Build 17142: arc lint + arc unit
dfaure requested review of this revision.Sep 28 2019, 11:18 AM
dfaure created this revision.
ervin accepted this revision.Sep 28 2019, 5:34 PM
This revision is now accepted and ready to land.Sep 28 2019, 5:34 PM

Any input on the (implicit) questions in the commit log? ;-)

ervin added a comment.Sep 29 2019, 6:58 AM

Ah, I didn't perceive it as a question. :-)

Don't we keep a shared ptr to those queries? I'd expect that or a weak pointer we can check at the job end in fact before getting in the lambdas.

dfaure added a comment.Oct 3 2019, 8:30 AM

It doesn't look like there is a null pointer we can check.

tests-units-presentation-inboxpagemodeltest(21269/21269) ?[32mPresentation::QueryTreeModel::~QueryTreeModel?[0m Presentation::QueryTreeModelBase(0x1f41d4a0) deleted
QDEBUG : InboxPageModelTest::shouldAddTasksInInbox() 17:26:02.005 tests-units-presentation-inboxpagemodeltest(21269/21269) ?[32mJobHandlerInstance::handleJobResult?[0m JobHandlerInstance(0x7584e0)
==21269== Invalid read of size 8
==21269==    at 0x49371C: operator() (std_function.h:704)
==21269==    by 0x49371C: operator() (livequery.h:238)
==21269==    by 0x49371C: std::_Function_handler<void (Akonadi::Collection const&), Domain::LiveQuery<Akonadi::Collection, QSharedPointer<Domain::DataSource> >::doFetch()::{lambda(Akonadi::Collection const&)#1}>::_M_invoke(std::_Any_data const&, Akonadi::Collection const&) (std_function.h:316)
==21269==    by 0x497278: operator() (std_function.h:706)
==21269==    by 0x497278: operator() (akonadilivequeryhelpers.cpp:190)
==21269==    by 0x497278: std::_Function_handler<void (), Akonadi::LiveQueryHelpers::fetchItemCollection(Akonadi::Item const&) const::{lambda(std::function<void (Akonadi::Collection const&)> const&)#1}::operator()(std::function<void (Akonadi::Collection const&)> const&) const::{lambda()#1}>::_M_invoke(std::_Any_data const&) (std_function.h:316)
==21269==    by 0x4F3D34: operator() (std_function.h:706)
==21269==    by 0x4F3D34: JobHandlerInstance::handleJobResult(KJob*) (jobhandler.cpp:52)
==21269==    by 0x94D8210: QtPrivate::QSlotObjectBase::call(QObject*, void**) (qobjectdefs_impl.h:394)
==21269==    by 0x9515E1F: void doActivate<false>(QObject*, int, void**) (qobject.cpp:3870)
==21269==    by 0x950FCBA: QMetaObject::activate(QObject*, QMetaObject const*, int, void**) (qobject.cpp:3929)
==21269==    by 0x6D19F4B: KJob::result(KJob*, KJob::QPrivateSignal) (moc_kjob.cpp:574)
==21269==    by 0x6D1A920: KJob::finishJob(bool) (kjob.cpp:107)
==21269==    by 0x47A572: CachingCollectionFetchJob::retrieveFromCache() (akonadicachingstorage.cpp:143)
==21269==    by 0x94D8210: QtPrivate::QSlotObjectBase::call(QObject*, void**) (qobjectdefs_impl.h:394)
==21269==    by 0x9508782: QMetaCallEvent::placeMetaCall(QObject*) (qobject.cpp:619)
==21269==    by 0x9509756: QObject::event(QEvent*) (qobject.cpp:1339)
==21269==    by 0x76F0172: QApplicationPrivate::notify_helper(QObject*, QEvent*) (qapplication.cpp:3690)
==21269==    by 0x76ED5FF: QApplication::notify(QObject*, QEvent*) (qapplication.cpp:3036)
==21269==    by 0x94C727F: QCoreApplication::notifyInternal2(QObject*, QEvent*) (qcoreapplication.cpp:1092)
==21269==    by 0x94C7BCB: QCoreApplication::sendEvent(QObject*, QEvent*) (qcoreapplication.cpp:1487)
==21269==    by 0x94C87DA: QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (qcoreapplication.cpp:1832)
==21269==    by 0x94C817B: QCoreApplication::sendPostedEvents(QObject*, int) (qcoreapplication.cpp:1691)
==21269==    by 0x9552EFE: postEventSourceDispatch(_GSource*, int (*)(void*), void*) (qeventdispatcher_glib.cpp:277)
==21269==    by 0x12739E86: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.5400.3)
==21269==    by 0x1273A22F: ??? (in /usr/lib64/libglib-2.0.so.0.5400.3)
==21269==    by 0x1273A2BB: g_main_context_iteration (in /usr/lib64/libglib-2.0.so.0.5400.3)
==21269==    by 0x9553633: QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (qeventdispatcher_glib.cpp:423)
==21269==    by 0x1B57F749: QXcbGlibEventDispatcher::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (qxcbeventdispatcher.cpp:143)
==21269==    by 0x94C786B: QCoreApplication::processEvents(QFlags<QEventLoop::ProcessEventsFlag>, int) (qcoreapplication.cpp:1345)
==21269==    by 0x954BB76: QTest::qWait(int) (qtestsupport_core.cpp:104)
==21269==    by 0x44B2C9: Testlib::TestHelpers::waitForEmptyJobQueue() (testhelpers.cpp:35)
==21269==    by 0x94D485F: QMetaMethod::invoke(QObject*, Qt::ConnectionType, QGenericReturnArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) const (qmetaobject.cpp:2294)
==21269==    by 0x5072003: QMetaMethod::invoke(QObject*, Qt::ConnectionType, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) const (qmetaobject.h:123)
==21269==    by 0x506A9B2: QTest::TestMethods::invokeTestOnData(int) const (qtestcase.cpp:950)
==21269==    by 0x506B281: QTest::TestMethods::invokeTest(int, char const*, QTest::WatchDog*) const (qtestcase.cpp:1140)
==21269==    by 0x506C5BC: QTest::TestMethods::invokeTests(QObject*) const (qtestcase.cpp:1481)
==21269==    by 0x506D115: QTest::qRun() (qtestcase.cpp:1919)
==21269==    by 0x506CBEC: QTest::qExec(QObject*, int, char**) (qtestcase.cpp:1808)
==21269==    by 0x41CF56: main (inboxpagemodeltest.cpp:579)
==21269==  Address 0x1f8a7da0 is 80 bytes inside a block of size 216 free'd
==21269==    at 0x4C2FA88: operator delete(void*) (vg_replace_malloc.c:576)
==21269==    by 0x49B7AD: operator delete (qsharedpointer_impl.h:173)
==21269==    by 0x49B7AD: ~QWeakPointer (qsharedpointer_impl.h:579)
==21269==    by 0x49B7AD: node_destruct (qlist.h:524)
==21269==    by 0x49B7AD: QList<QWeakPointer<Domain::LiveQueryInput<Akonadi::Collection> > >::dealloc(QListData::Data*) [clone .isra.21] (qlist.h:904)
==21269==    by 0x49BF62: Akonadi::LiveQueryIntegrator::~LiveQueryIntegrator() (akonadilivequeryintegrator.cpp:50)
==21269==    by 0x49BFE8: Akonadi::LiveQueryIntegrator::~LiveQueryIntegrator() (akonadilivequeryintegrator.cpp:53)
==21269==    by 0x47328D: non-virtual thunk to Akonadi::TaskQueries::~TaskQueries() (in /d/kde/build/5/extragear/pim/zanshin/tests/units/presentation/tests-units-presentation-inboxpagemodeltest)
==21269==    by 0x41D820: destroy (qsharedpointer_impl.h:163)
==21269==    by 0x41D820: QSharedPointer<Domain::Task>::deref(QtSharedPointer::ExternalRefCountData*) [clone .part.35] (qsharedpointer_impl.h:468)
==21269==    by 0x425E3D: deref (qsharedpointer_impl.h:466)
==21269==    by 0x425E3D: deref (qsharedpointer_impl.h:463)
==21269==    by 0x425E3D: ~QSharedPointer (qsharedpointer_impl.h:321)
==21269==    by 0x425E3D: Presentation::InboxPageModel::~InboxPageModel() (inboxpagemodel.h:35)
==21269==    by 0x428853: InboxPageModelTest::shouldAddTasksInInbox() (inboxpagemodeltest.cpp:212)
==21269==    by 0x94D485F: QMetaMethod::invoke(QObject*, Qt::ConnectionType, QGenericReturnArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) const (qmetaobject.cpp:2294)
==21269==    by 0x5072003: QMetaMethod::invoke(QObject*, Qt::ConnectionType, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) const (qmetaobject.h:123)
==21269==    by 0x506A7B4: QTest::TestMethods::invokeTestOnData(int) const (qtestcase.cpp:941)
==21269==    by 0x506B281: QTest::TestMethods::invokeTest(int, char const*, QTest::WatchDog*) const (qtestcase.cpp:1140)
==21269==    by 0x506C5BC: QTest::TestMethods::invokeTests(QObject*) const (qtestcase.cpp:1481)
==21269==    by 0x506D115: QTest::qRun() (qtestcase.cpp:1919)
==21269==    by 0x506CBEC: QTest::qExec(QObject*, int, char**) (qtestcase.cpp:1808)
==21269==    by 0x41CF56: main (inboxpagemodeltest.cpp:579)
==21269==  Block was alloc'd at
==21269==    at 0x4C2EA6A: operator new(unsigned long) (vg_replace_malloc.c:334)
==21269==    by 0x4B88D9: create (qsharedpointer_impl.h:274)
==21269==    by 0x4B88D9: create<> (qsharedpointer_impl.h:445)
==21269==    by 0x4B88D9: bind<QSharedPointer<Domain::DataSource>, std::function<void(const std::function<void(const Akonadi::Collection&)>&)>, Akonadi::TaskQueries::findDataSource(Domain::Task::Ptr) const::<lambda(const Akonadi::Collection&)> > (akonadilivequeryin
ervin added a comment.Oct 3 2019, 9:30 AM

This is annoying... I'm not sure how to address it properly for now...

dfaure closed this revision.Nov 23 2019, 10:13 AM