d1vanov/libquentier

Implement infrastructure for testing of synchronization logic

Closed this issue · 6 comments

Synchronization logic is quite complex and definitely needs some regression tests. However, before we can have the tests, we need the infrastructure for them i.e. we need some way to fake the Evernote servers communicating with the local app. I see several possible solutions:

  1. Add INoteStore interface which would be implemented by NoteStore (for production synchronization) and FakeNoteStore (for tests). The injection of a particular note store would then need to happen in runtime. It means some switch would be needed to tell libquentier it needs to inject the fake note store instead of the real one for tests. It can be a dedicated API method or it can be something better hidden, for example, examining some environment variable like LIBQUENTIER_USE_FAKE_NOTE_STORE_FOR_SYNCHRONIZATION_TESTING. Or it might examine the value within some settings file instead of the environment variable.
  2. Use some trick with preloading some shared library "catching" the calls to QEverCloud's methods. On Linux it could work with the help of LD_PRELOAD but not sure if there are equivalents on Windows and Mac.
  3. Implement fake server on the local host with which QEverCloud would communicate. Most complex of all variants as it would require decoding and encoding of data from/to Thrift binary format. QEverCloud currently doesn't export the stuff related to that.

After a month since the work on this ticket has started I finally got one simple sync test passing. Think I'll try to implement a couple more simple tests and leave more complex ones for the future.

In contradiction with my previous comment I just couldn't help myself and started to implement each and every sync logic test I could imagine to be useful. It's a pity it takes so much time but I believe it's valuable enough to secure the lack of future sync regressions.

I have several incremental sync tests left to implement, then I need to add a check for post-sync persistence (it should contain all the relevant info required to run the next incremental sync properly), then sync conflict resolution logic and finally Evernote API rate limits processing. Not sure I'd be implementing all of this right here and right now but at least that's what needs to be done eventually.

A bit of clarification to the previous comment: the kinds of incremental sync left to cover with tests are as follows:

  1. When something (notebook, note, tag, saved search) was expunged (permanently removed) from the Evernote account and during the sync the corresponding data item needs to be removed from the local storage database as well.
  2. When conflicting changes are found during the sync - in case of notebooks, tags and saved searches the conflict is "resolved" via favoring the Evernote server's version. In case of notes both versions are saved: the Evernote server's version is saved within the normal note and the local version is saved as a new note specifically marked as conflicting note.

The number of tests for sync logic has reached 61 now. That is, if you don't count 41 more tests for FullSyncStaleDataItemsExpunger which were implemented about a year ago. The only kind of test I would still like to add to this is the one for Evernote API rate limits. Oddly enough, in all my live experiments I've never encountered it in practice and hence couldn't test its handling. Due to this it can take from a couple of days to another couple of weeks to implement the test and fix the found mistakes in API rate limits handling. But that would surely be the last thing to do within this issue.

I got really close to finishing this: with the latest commit there are 73 tests for the synchronization logic and now they cover almost everything related to downloading stuff from Evernote servers into the app's local storage database. The most part of sending locally added or changed items back to Evernote is already covered, what's left to check is the handling of API rate limits exceeding on attempts to send new/modified stuff back to Evernote.

Finally finished this issue, merged the results to development branch.

It took me two months to implement a comprehensive suite of tests for the synchronization logic. This kind of work is typically never noticed by the end users since they just expect things to work and not break over time. Being an end user for a lot of software I know just how frustrating it is when something that used to work ceases to work. Implementing regression tests requires surprisingly much effort but it really pays off over time since it can dramatically reduce the amount of regressions and hence reduce the number of end users frustrated by these regressions.

Believe it or not, there is still a small room for more tests on the synchronization logic: I can think of a kind of integration test for full sync performed instead of incremental sync when the account hasn't been synchronized with Evernote for a long time. And one more testable thing is repeating the download of stuff from Evernote if something was added or changed while the app was sending the local changes back to Evernote after the initial download of stuff from there. Since both of these things are quite rarely occurring edge cases which also should work correctly given the results of other tests and some common sense, I decided not to implement these tests now. Perhaps I should create issues for the sake of having more "good first issues" in case someone would like to contribute one of these tests.