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
Branch
2019_collectionscheduler_tests
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10725
Build 10743: arc lint + arc unit
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
325 ↗(On Diff #55695)

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

329 ↗(On Diff #55695)

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
329 ↗(On Diff #55695)

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.