[Evolution-hackers] Isolated unit test status report

[Tristan <tristanvb openismus com>]

Since I spent some time today debugging the situation, I wanted to just
share my findings so that we can hopefully fix up the remaining messy
parts, piece by piece.

There are a few remaining problems which cause the test cases to
not consistently pass, so let's run through them in a random order
(there may be more problems but these are the ones known to me
at this time).

A.) The registry and factory services do not properly shut-down
    between test cases.

This is probably the single most difficult and important thing
to fix regarding the test cases. If you take a quick look at
the test fixture[0] used to test client APIs, you will notice
a fat FIXME comment near the top.

This refers to us being incapable to properly shutdown the
GTestDBus object used for testing. To work around this we
have the code #ifdef'd into GLOBAL_DBUS_DAEMON segments
(so we test all tests in a given suite with the same D-Bus
daemon instead of cleaning up between each case).

This indicates that somewhere, in the client or server, we
are leaking references to the GDBusConnections. I've tested
this in raw GIO (and added a test case there) to ensure it
is indeed possible to stop/start the GTestDBus between tests,
the problem is clearly an ugly bug to be fixed in EDS.

B.) Leaking ref counts on EBookClient

This is rather a prerequisite of 'A', since the services wait
for EClients to cleanly disconnect, there's no way we're going
to get the service to cleanly shutdown between tests.

There is a relevant bug report to this here[1].

C.) Race conditions when adding ESources

This is discussed in the recent thread here[2]. Because
we are still unsure whether the ESource has made it into
the addressbook factory's local ESourceRegistry instance
at the time we ask the factory to create an addressbook,
we fall into a trap sometimes where the addressbook
fails to be created.

D.) Race conditions when removing ESources

This might be another consequence of 'A', or perhaps that
if 'A' were solved, we would not have ever noticed this
race condition (so let's be thankful that 'A' is broken for now :D).

Currently, test cases behave a little differently if
EDS is installed or not installed. This is because the
ESourceRegistryServer loads the cache-reaper registry
module in the case that EDS is installed (I've created
a patch today which ensures that the in tree cache-reaper
module directory is used instead of the installed directory).

What is interesting about this, is that if the cache-reaper
is present, tests start to fail.

This is due to a race condition in e_client_remove_sync(),
this call is made at the end of a test case.. which results
in the ESource being removed from the ESourceRegistry server.

What happens when the cache-reaper module is loaded, is that
the 'test-address-book' source is removed /some time after/
e_client_remove_sync() completes... sometimes this happens
right in the middle of the following test case.

The result is that we fail in a side-effect, sometimes we
find ourselves in one of the test cases adding a contact
to an addressbook. The test code will generally:

   - Add a contact with e_book_client_add_contact_sync()
   - Check the contact is there with
     e_book_client_get_contact_sync()

What happens is that somewhere in between these two
steps, the cache-reaper module steps in and finally
gets the message (from the previous test case) that this
addressbook needs to be moved to the trash... the
result is that we fail to fetch the newly added
contact.

Note that we *never* fail to fetch a newly added contact
if the cache-reaper module is not present in the testing
environment.

E.) A fresh build of EDS will not pass tests,
    after make install && make uninstall tests pass

This is because make uninstall does not remove
the module directories created by make install.

Tests fail because the registry service bails out
when e_module_load_all_in_directory() fails for
the registry modules.

This is fixed with the patch (not committed) which 
I created today. This is not ideal because my patch
redirects the registry modules to use the in-tree
cache-reaper module directory. (meaning we invariably face
the race condition issues described in 'D').

Cheers,
      -Tristan

[0]:http://git.gnome.org/browse/evolution-data-server/tree/tests/test-server-utils/e-test-server-utils.c
[1]:https://bugzilla.gnome.org/show_bug.cgi?id=693461
[2]:https://mail.gnome.org/archives/evolution-hackers/2013-January/msg00027.html