Create a mock LDAP server and some tests
Categories
(MailNews Core :: LDAP Integration, task)
Tracking
(Not tracked)
People
(Reporter: darktrojan, Assigned: darktrojan)
Details
Attachments
(3 files)
|
(deleted),
patch
|
mkmelin
:
review+
|
Details | Diff | Splinter Review |
|
(deleted),
patch
|
mkmelin
:
review+
|
Details | Diff | Splinter Review |
|
(deleted),
patch
|
mkmelin
:
review+
|
Details | Diff | Splinter Review |
Most of the LDAP code has zero test coverage. In this bug I'll create a mock server and some basic functionality tests. Then I'll fix offline replication of LDAP address books, which I suspected was broken, and can now prove with a test.
|
Assignee | |
Comment 1•5 years ago
|
||
We created this test in the wrong place. It should be under mail/, where the code it tests is. I also found a pointless moz.build line so I removed it and the same line in another file.
|
|
Assignee | |
Comment 2•5 years ago
|
||
This part adds the mock server and two tests – one that tests the address book UI works correctly when searching for a contact, and one that tests that offline replication works.
|
|
Assignee | |
Comment 3•5 years ago
|
||
This fixes the offline replication, apart from a weird unknown failure when run in debug mode.
There's a lot going on here:
- The replication directory is now instantiated directly instead of through the AB manager. This prevents the AB manager from knowing about it and doing unwanted things like caching.
- When making a second or subsequent replication, we write to a new file and only then, if successful, delete the old one and move the new one into place. This saves a lot of hassle moving things around.
- The SQLite connection needs to be closed before files can move around. I've done this by firing an observer service notification requesting closure and (because it's async) another one responding when it has closed.
- I've used MozPromise to handle the async stuff in C++. This is completely new to me but it seems to work.
|
||
Updated•5 years ago
|
|
|
||
Comment 4•5 years ago
|
||
Comment on attachment 9119927 [details] [diff] [review] 1608304-pt2-create-ldap-tests-1.diff Review of attachment 9119927 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/components/addrbook/content/addressbook.js @@ +141,5 @@ > ResultsPaneSelectionChanged(); > }, > onCountChanged(total) { > SetStatusText(total); > + window.dispatchEvent(new CustomEvent("countChanged")); event names are usually alllowercase, and no past tense Maybe countchange or abviewcountchange? Might as well put total into the event detail new CustomEvent("countchange", detail: { total }); ::: mailnews/addrbook/test/LDAPServer.jsm @@ +1,5 @@ > +/* This Source Code Form is subject to the terms of the Mozilla Public > + * License, v. 2.0. If a copy of the MPL was not distributed with this > + * file, you can obtain one at http://mozilla.org/MPL/2.0/. */ > + > +var EXPORTED_SYMBOLS = ["LDAPServer"]; inside a jsm, so you can make this const @@ +6,5 @@ > +var PRINT_DEBUG = false; > + > +var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm"); > + > +var serverSocket; looks like this should just be a member of LDAPServer @@ +7,5 @@ > + > +var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm"); > + > +var serverSocket; > +var LDAPServer = { please add some more documentation to this jsm while it's fresh in memory.
|
|
||
Comment 5•5 years ago
|
||
Comment on attachment 9119930 [details] [diff] [review] 1608304-pt3-fix-replication-1.diff Review of attachment 9119930 [details] [diff] [review]: ----------------------------------------------------------------- LGTM thx! r=mkmelin
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/d85625eb94fb
part 1 - Move test of address book UI from mailnews/addrbook to mail/components/addrbook; r=mkmelin
https://hg.mozilla.org/comm-central/rev/f39320031868
part 2 - Create a mock LDAP server and some tests; r=mkmelin
https://hg.mozilla.org/comm-central/rev/46ed6cdc30b5
part 3 - Fix offline replication of LDAP address books; r=mkmelin
|
|
Assignee | |
Updated•5 years ago
|
Description
•