After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 678811 - Fix eds backend unit tests
Fix eds backend unit tests
Status: RESOLVED FIXED
Product: folks
Classification: Platform
Component: e-d-s backend
git master
Other Linux
: Normal normal
: Unset
Assigned To: folks-maint
folks-maint
Depends on:
Blocks:
 
 
Reported: 2012-06-25 19:28 UTC by Jeremy Whiting
Modified: 2012-06-25 22:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
edsunittests branch squashed into one patch. (10.70 KB, patch)
2012-06-25 19:28 UTC, Jeremy Whiting
needs-work Details | Review

Description Jeremy Whiting 2012-06-25 19:28:42 UTC
Created attachment 217237 [details] [review]
edsunittests branch squashed into one patch.

Unit tests for the eds backend were disabled shortly before folks 0.7.1 release when porting to eds 3.5.3 api changes.  They need to be re-enabled and fixed to work with the eds api changes.

The branch at http://git.gnome.org/browse/folks/log/?h=edsunittests fixes the eds backend unit tests.  I've attached a patch for review that has the same changes as that branch.
Comment 1 Philip Withnall 2012-06-25 21:00:02 UTC
Review of attachment 217237 [details] [review]:

A number of trivial style problems to fix, a comment to be added, and then this can be committed to master. Please squash a few of the commits together first though so that they’re all self-contained. Don’t forget to update the NEWS file and link to this bug report from the final commit! Thanks.

::: tests/lib/eds/backend.vala
@@ +105,3 @@
           this._addressbook.open_sync (false, null);
           this._addressbook_name =
+            this._addressbook.get_source().get_uid ();

Missing space before the first ‘()’. Also, why is the UID being used for the address book name?

@@ +117,3 @@
   public void set_as_default ()
     {
+      this._source_registry.set_default_address_book(this._source);

Missing space before the ‘(’. There are more of them below, too.

@@ +135,2 @@
       if (is_default)
+        set_as_default();

Please explicitly use ‘this.’ when referring to things in the same class.

@@ +267,1 @@
+      this._addressbook.remove.begin (null, (o, r) =>

There should probably be a comment here explaining that we’re using the async method because the sync method hangs, and linking to a bug number if one exists.

@@ +280,3 @@
             {
+              GLib.warning ("Unable to remove addressbook %s because: %s\n",
+              this._addressbook_name, e.message);

The second line should be indented 4 spaces more than the first because it’s wrapped. Same with the other warning above.