Add test for CollectionScheduler, code cleanups, bugfixes.
ClosedPublic

Authored by dfaure on Apr 7 2019, 5:01 PM.

Details

Summary

Wrote a unittest just for fun. Found lots of interesting things...

  • Methods called in the wrong thread, leading to QTimer warnings
  • Unscheduling all collections with the same "next time" when one collection is removed
  • for() loops with if+return, where find_if() is much more readable
  • Unnecessary detaching (non-const lowerBound)
  • All this will break in 2038 (32 bit time in seconds), will fix in next commit
Test Plan

bin/collectionschedulertest

Diff Detail

Repository
R165 Akonadi
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
dfaure created this revision.Apr 7 2019, 5:01 PM
Restricted Application added a project: KDE PIM. · View Herald TranscriptApr 7 2019, 5:01 PM
dfaure requested review of this revision.Apr 7 2019, 5:01 PM
dvratil requested changes to this revision.Apr 9 2019, 9:19 AM
dvratil added inline comments.
src/server/collectionscheduler.cpp
326–327

Could you please also fix this to use QMutexLocker and move it to the top of the method?

330

This is also broken, as below we are iterating over un-protected collections list...

This revision now requires changes to proceed.Apr 9 2019, 9:19 AM
dfaure marked an inline comment as done.Apr 11 2019, 6:45 AM
dfaure added inline comments.
src/server/collectionscheduler.cpp
330

It's local copy on stack, seems fine to me.

dfaure updated this revision to Diff 55964.Apr 11 2019, 6:47 AM

use mutexlocker

dvratil accepted this revision.Apr 12 2019, 8:27 AM
This revision is now accepted and ready to land.Apr 12 2019, 8:27 AM
This revision was automatically updated to reflect the committed changes.