GNOME Bugzilla – Bug 658327
Folks backend tests spew garbage into EDS gconf file
Last modified: 2012-06-29 15:21:08 UTC
Created attachment 195754 [details] garbage spewed into the eds gconf file I've had serious problems with failing tests for the eds backend for a while. The error comes out in most cases as: folks-WARNING **: Error preparing persona store 'eds:local://other': Couldn't open address book ?local://other?: Cannot open book: db error 0x2 (No such file or directory) This is happening because Eds.Backend sees the "local://test" and "local://other" backends and decides to open each as its own PersonaStore. Since we clear out these temporary abooks after each test, and only the link-personas test creates local://other, the E.BookClient fails to open "local://other". I noticed that tons of entries for both get added to ~/.gconf/apps/evolution/addressbook/%gconf.xml each time I run the eds tests. See the attached file. So this may be at least partially a libebook bug, but it could be useful to debug it from both sides.
Raúl and Philip haven't hit this problem - but do you two have similar garbage in the same gconf file? Matthew, do you know why this might be happening? I'm using the latest git master version of eds.
(In reply to comment #1) > Raúl and Philip haven't hit this problem - but do you two have similar garbage > in the same gconf file? I have hit the problem, but I never got time to debug it.
(In reply to comment #0) (...) > I noticed that tons of entries for both get added to > ~/.gconf/apps/evolution/addressbook/%gconf.xml each time I run the eds tests. > See the attached file. Hrmm, writing to your home dir? That shouldn't (theoretically) happen since we do this: eds_tmpdir=$(mktemp -d) export XDG_DATA_HOME=$eds_tmpdir/.local export XDG_CACHE_HOME=$eds_tmpdir/.cache export XDG_CONFIG_HOME=$eds_tmpdir/.config before starting the new session bus (and having e-addressbook-factory launched). Unless e-addresbook-factory honors some other env var we don't know of.
My guess is the extra GConf entries are a side-effect of the patch for bug #651226.
(In reply to comment #4) > My guess is the extra GConf entries are a side-effect of the patch for bug > #651226. Those patches seem to be against EBook and we only use EBookClient in folks.
(In reply to comment #5) > (In reply to comment #4) > > My guess is the extra GConf entries are a side-effect of the patch for bug > > #651226. > > Those patches seem to be against EBook and we only use EBookClient in folks. Similar logic exists in e_client_util_get_system_source().
For our tests we prepare address books with the following steps: - new ESourceGroup - new ESource - add ESource to ESourceGroup - get SourceList from BookClient - add ESourceGroup to the SourceList - sync the SourceList I'll check if there is similar logic for that path.
git bisect points to this as the problem-causer: 364e730e2cbdc22f786ad5d3e8fe9bc95137b6ad is the first bad commit commit 364e730e2cbdc22f786ad5d3e8fe9bc95137b6ad Author: Raul Gutierrez Segales <rgs@collabora.co.uk> Date: Wed Aug 31 19:57:07 2011 +0100 e-d-s: add test to link personas from different stores This test checks what was fixed in: https://bugzilla.gnome.org/show_bug.cgi?id=657635
Merged this: commit 3959555f24f7696bcda9af7e265682997c1076d0 Author: Raul Gutierrez Segales <rgs@collabora.co.uk> Date: Wed Sep 7 12:41:09 2011 +0100 e-d-s: free resources properly between tests cases on link-persona test unit It is a work around, but so far I haven't seen the problem again.
Created attachment 195881 [details] [review] Kill gconfd after running a test I spoke too soon :) Now, with this patch the problem is *really* gone :) We were not killing GConf so our efforts to clean up the entries generated by each test execution were in vain because gconf-d would then sync to disc and re-write what we thought had been cleaned up. So hopefully this makes tests more deterministic.
(In reply to comment #10) > Created an attachment (id=195881) [details] [review] > Kill gconfd after running a test > > I spoke too soon :) Now, with this patch the problem is *really* gone :) > > We were not killing GConf so our efforts to clean up the entries generated by > each test execution were in vain because gconf-d would then sync to disc and > re-write what we thought had been cleaned up. So hopefully this makes tests > more deterministic. It works OK as long as you comment out the link-personas test. In that case, the gconf file doesn't get spewed to, the tests pass, etc. However, I still hit the described problems with the link-personas test. Are you sure you didn't comment it out when you tested this last patch?
(In reply to comment #11) > (In reply to comment #10) > > Created an attachment (id=195881) [details] [review] [details] [review] > > Kill gconfd after running a test > > > > I spoke too soon :) Now, with this patch the problem is *really* gone :) > > > > We were not killing GConf so our efforts to clean up the entries generated by > > each test execution were in vain because gconf-d would then sync to disc and > > re-write what we thought had been cleaned up. So hopefully this makes tests > > more deterministic. > > It works OK as long as you comment out the link-personas test. In that case, > the gconf file doesn't get spewed to, the tests pass, etc. > > However, I still hit the described problems with the link-personas test. Are > you sure you didn't comment it out when you tested this last patch? Either way, it's worth merging, so please do so. I'll see if I can dig up why I still hit problems, so let's keep this bug open until I work that out.
Merged: commit b9f391ab8c4170772b27c81b0e1ce651ec37360d Author: Raul Gutierrez Segales <rgs@collabora.co.uk> Date: Wed Sep 7 15:52:27 2011 +0100 e-d-s: kill gconfd-2 after running a test I thought we were properly cleaning up the GConf entries generated by each test run but it turns out that sometimes gconfd-2 exits /after/ we've supposedly nuked it's directory. So before cleaning up GConf's files we now kill gconfd-2 to prevent it from sync-ing to disc afterwards. Helps: https://bugzilla.gnome.org/show_bug.cgi?id=658327
Comment on attachment 195881 [details] [review] Kill gconfd after running a test (Marking the patch as committed.) What else needs doing here?
(In reply to comment #14) > (From update of attachment 195881 [details] [review]) > (Marking the patch as committed.) > > What else needs doing here? I guess we were waiting for some feedback testing this out and see if it actually fixes the problem. But we could close it and reopen if necessary.
(In reply to comment #15) > (In reply to comment #14) > > (From update of attachment 195881 [details] [review] [details]) > > (Marking the patch as committed.) > > > > What else needs doing here? > > I guess we were waiting for some feedback testing this out and see if it > actually fixes the problem. But we could close it and reopen if necessary. Let's do that.
(In reply to comment #15) > (In reply to comment #14) > > (From update of attachment 195881 [details] [review] [details]) > > (Marking the patch as committed.) > > > > What else needs doing here? > > I guess we were waiting for some feedback testing this out and see if it > actually fixes the problem. But we could close it and reopen if necessary. This bug was basically two issues: 1. most of our EDS tests were failing for me consistently due to a missing "local://other" address book 2. there was garbage getting spewed into the gconf file The second issue still exists (hence re-opening), but the first issue (which is fixed for me now) was much more important (which is why I'm making this low priority). My best guess is that we're not quite closing the BookClients correctly for some tests (I don't get the absolute flood of entries in my GConf file that I used to, but there are still some).
(In reply to comment #17) > (In reply to comment #15) > > (In reply to comment #14) > > > (From update of attachment 195881 [details] [review] [details] [details]) > > > (Marking the patch as committed.) > > > > > > What else needs doing here? > > > > I guess we were waiting for some feedback testing this out and see if it > > actually fixes the problem. But we could close it and reopen if necessary. > > This bug was basically two issues: > > 1. most of our EDS tests were failing for me consistently due to a missing > "local://other" address book > 2. there was garbage getting spewed into the gconf file Scratch that. I'm hitting 1. again (even after a mostly-clean install) :-/ Bumping to high priority.
(In reply to comment #18) > (In reply to comment #17) > > (In reply to comment #15) > > > (In reply to comment #14) > > > > (From update of attachment 195881 [details] [review] [details] [details] [details]) > > > > (Marking the patch as committed.) > > > > > > > > What else needs doing here? > > > > > > I guess we were waiting for some feedback testing this out and see if it > > > actually fixes the problem. But we could close it and reopen if necessary. > > > > This bug was basically two issues: > > > > 1. most of our EDS tests were failing for me consistently due to a missing > > "local://other" address book > > 2. there was garbage getting spewed into the gconf file > > Scratch that. I'm hitting 1. again (even after a mostly-clean install) :-/ > > Bumping to high priority. Sorry for the noise. This was a left-over effect from importing my Evo settings from my last machine. In fact, even problem 2. seems to be gone at this point, so I'm closing this.
(In reply to comment #19) > Sorry for the noise. This was a left-over effect from importing my Evo settings > from my last machine. > > In fact, even problem 2. seems to be gone at this point, so I'm closing this. Nope, it was still busted. But I /finally/ did track down what it was. My system-installed gconf 2.32.4 was configured to use ORBit for its IPC. And the jhbuilt gconf uses D-Bus by default. In this configuration, you can have two instances of gconf running at once (which causes general confusion). Possibly related to that last detail is the way I never manged to get the D-Bus-using GConf to actually honor the set GCONF_DEFAULT_SOURCE_PATH (which is why I got garbage entries in my personal ~/.gconf tree). It's likely related to this error message: GConf Error: Configuration server couldn't be contacted: D-BUS error: Method "GetDefaultDatabase" with signature "" on interface "org.gnome.GConf.Server" doesn't exist (GetDefaultDatabase looks to be related to the function that cares about GCONF_DEFAULT_SOURCE_PATH: gconf_engine_get_default ()). So, my work-around is to build my development GConf against ORBit until the GConf packages in Debian experimental are built against D-Bus. So far, everything seems to be consistent with it and I get zero garbage in my personal gconf data tree.
This isn't solved after all. And it doesn't appear that it was a matter of clashing system-installed and jhbuild-installed instances of gconf; two instances of gconfd are running at once while our tests run. I'm not sure this is necessarily a problem though. One very odd thing in all of this is the fact that these entries don't even appear in gconf-editor. The respective %gconf.xml file fills up but gconf-editor only shows the non-test address books listed.
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > My guess is the extra GConf entries are a side-effect of the patch for bug > > > #651226. > > > > Those patches seem to be against EBook and we only use EBookClient in folks. > > Similar logic exists in e_client_util_get_system_source(). Matthew, do you think this could be causing our trouble? Is it possible this is partly/wholly due to a bug in EDS?
Hard to say really. I'm not familiar with what your test harness is doing. FWIW, I have some big API breaks coming to libedataserver after 3.4. We'll be simultaneously dropping GConf, ESourceList, ESourceGroup, and URI usage in favor of a simpler system based on plain text files that will hopefully make E-D-S testing much easier.
After declaring this issue yet again resolved in bug#668415 comment#22, this bug is back (at least for my setup). I'll be disabling the affect tests (which aren't too critical) in the meantime, but that's not a long-term fix.
Matthew, What's the best way with the new eds api changes to make a temporary E.Source for our unit tests? We used to create a new SourceGroup, create a new Source, etc. as Raul noted in comment 7, but we briefly discussed setting XDG_DATA_DIR to some temporary path for our unit tests and having a .source file in there. Could you elaborate on what that .source file should have in it? I see some rw-sources and ro-sources in /opt/gnome/share/evolution-data-server-3.6/ and assume it should be similar to what's in there. What env vars do we need to set to use our custom source XDG_DATA_DIR or something else? I'd like to get the eds backend unit tests back up and working soonish. thanks
The idea is just to set up a sandbox environment for the test to run in. Doesn't need to be as isolated as a chroot, but just so it won't harm any real Evolution accounts and you can easily clean up afterward. The test harness should probably create a "test" folder somewhere, and then override the various user-specific XDG base directories through environment variables. Maybe something like: Set $XDG_DATA_HOME to ".../test/share" Set $XDG_CACHE_HOME to ".../test/cache" Set $XDG_CONFIG_HOME to ".../test/config" And then start the Evolution-Data-Server D-Bus services in such a way that they pick up those environment variables. Easiest way to create a temporary ESource for automated testing is just to write a key file ahead of time and copy it to $XDG_CONFIG_HOME/evolution/sources. The key file will be exported over D-Bus and made available through ESourceRegistry. A minimal key file for a local address book looks like this: [Data Source] DisplayName=Test Address Book Parent=local-stub [Address Book] BackendName=local Additional file format details are here: https://live.gnome.org/Evolution/ESourceFileFormat The key file name should have a ".source" extension. So if you name it, say, "test-address-book.source" then the "test-address-book" part will become its unique ID and you can access it through ESourceRegistry like so: source = e_source_registry_ref_source (registry, "test-address-book"); The rest of the client-side ESource API is documented here: mbarnes.fedorapeople.org/account-mgmt/docs/libedataserver/
Travis, Now that eds is no longer using gconf since the api changes, and now that we have ported our tests to work with the api changes do you still hit either of the issues in this bug? If not I think it should be safe to close this. Re-open if you still hit issues but I don't see any gconf files even getting created here when I run the unit tests.