1970018 - Get the xpcshell tests in mailnews/local passing with panorama.

Status: ASSIGNED

(MailNews Core :: Database, task)

Description

[Ben Campbell]

Just a place for me to land stuff while I'm going through getting the mailnews/local tests up and running with the new global db.

To run these tests:

$ ./mach xpcshell-test --setpref mail.panorama.enabled=true comm/mailnews/local/test/unit/test_.....

Comment 1

[Ben Campbell]

Attachment: Bug 1970018 - Update StoreIndexer to use nsIMsgDatabase.attachHdr(). r=#thunderbird-back-end-reviewers

Comment 2

[Ben Campbell]

Attachment: Bug 1970018 - Fix keyword handling in Message::Get/SetStringProperty(). r=#thunderbird-back-end-reviewers

Comment 3

[Ben Campbell]

Attachment: Bug 1970018 - Implement PerFolderDatabase message delete functions. r=#thunderbird-back-end-reviewers

Comment 4

[Ben Campbell]

Attachment: Bug 1970018 - Don't rely upon data on deleted msgHdr in test_perFolderDatabase.js. r=#thunderbird-back-end-reviewers

Comment 5

[Ben Campbell]

try run including those patches here:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=cddaca39165d0f87848f843632720da556a247aa

Comment 6

[Ben Campbell]

Attachment: Bug 1970018 - Pull panorama message caching into database layer. r=#thunderbird-back-end-reviewers

Comment 7

[Ben Campbell]

Attachment: Bug 1970018 - Panorama version of AddMessageBatch() to use default timestamp for missing "Date:" header. r=#thunderbird-back-end-reviewers

Comment 8

[Ben Campbell]

Attachment: Bug 1970018: Treat unread messages as new when adding to PerFolderDatabase. r=#thunderbird-back-end-reviewers

Comment 9

[Ben Campbell]

Attachment: Bug 1970018 - Handle extra "Message-Id:" whitespace in ParseMsgHeaders(). r=#thunderbird-back-end-reviewers

Comment 10

[Ben Campbell]

Current status:

PASS test_bug457168.js
FAIL test_duplicateKey.js           # missing expungedBytes support (and compaction?).
FAIL test_fileName.js               # n/a (installs msf files)
PASS test_folderLoaded.js
FAIL test_folderParse.js            # n/a?
FAIL test_localFolder.js            # folderdiscovery issues?
PASS test_localServerFromPrefs.js
PASS test_mailboxContentLength.js
PASS test_mailboxProtocol.js
PASS test_mailboxURL.js
PASS test_mailStoreCompact.js
FAIL test_mailStoreEnvData.js       # folder reparse test broken - deleting .msf doesn't affect panorama.
PASS test_mboxMalformed.js
PASS test_msgCopy.js
PASS test_msgIDParsing.js
PASS test_noTop.js
PASS test_noUidl.js
FAIL test_nsIMsgLocalMailFolder.js  # moving folders is bolloxed?
PASS test_nsIMsgParseMailMsgState.js
FAIL test_nsIMsgPluggableStore.js   #folderdiscovery issues
PASS test_over2GBMailboxes.js
FAIL test_over4GBMailboxes.js         # no error, just doesn't finish
PASS test_pop3AuthMethods.js
PASS test_Pop3Channel.js
PASS test_pop3Client.js
PASS test_pop3Download.js
PASS test_pop3DownloadTempFileHandling.js
PASS test_pop3Duplicates.js
FAIL test_pop3FilterActions.js       # nsIMsgDatabase.summaryValid missing?
PASS test_pop3Filters.js
PASS test_pop3GetNewMail.js
PASS test_pop3GSSAPIFail.js
FAIL test_pop3MoveFilter2.js
FAIL test_pop3MoveFilter.js
FAIL test_pop3MultiCopy2.js           # relying on detached messageSize/lineCount?
FAIL test_pop3MultiCopy.js            # relying on detached messageSize/lineCount?
PASS test_pop3Password3.js
PASS test_pop3PasswordFailure_rfc1939.js
PASS test_pop3PasswordFailure_rfc2449.js
PASS test_pop3PasswordFailure_rfc5034.js
PASS test_pop3Password.js
PASS test_pop3Proxy.js
PASS test_pop3Pump.js
???? test_pop3ServerBrokenCRAMDisconnect.js  # skip rule kicked in
???? test_pop3ServerBrokenCRAMFail.js        # skip rule kicked in
PASS test_pop3ServerFromPrefs.js
PASS test_preview.js
PASS test_saveMessage.js
FAIL test_streamHeaders.js   # tries to delete msf file
FAIL test_undoDelete.js      # tries to remove message with key nsMsgKey_None
PASS test_verifyLogon.js
PASS test_viewSortBySize.js

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a97f894c5eb5
Update StoreIndexer to use nsIMsgDatabase.attachHdr(). r=darktrojan
https://hg.mozilla.org/comm-central/rev/810d1fe9f49c
Implement PerFolderDatabase message delete functions. r=darktrojan
https://hg.mozilla.org/comm-central/rev/7de0c95bbd0e
Don't rely upon data on deleted msgHdr in test_perFolderDatabase.js. r=darktrojan
https://hg.mozilla.org/comm-central/rev/ffb4b5ddf32f
Fix keyword handling in Message::Get/SetStringProperty(). r=darktrojan

Comment 12

[Ben Campbell]

Attachment: Bug 1970018 - Reduce some panorama includes using forward declarations. r=#thunderbird-back-end-reviewers

Comment 13

[Ben Campbell]

Attachment: Bug 1970018 - Add some mozStorageStatementScoper to panorama. r=#thunderbird-back-end-reviewers

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/5fe03645189f
Pull panorama message caching into database layer. r=darktrojan
https://hg.mozilla.org/comm-central/rev/f5d1e53d3014
Panorama version of AddMessageBatch() to use default timestamp for missing "Date:" header. r=edicharry
https://hg.mozilla.org/comm-central/rev/0cb5882c2e8f
Handle extra "Message-Id:" whitespace in ParseMsgHeaders(). r=edicharry
https://hg.mozilla.org/comm-central/rev/38521d42c668
Reduce some panorama includes using forward declarations. r=darktrojan
https://hg.mozilla.org/comm-central/rev/bccc5030d068
Add some mozStorageStatementScoper to panorama. r=darktrojan

Comment 15

[Ben Campbell]

Attachment: Bug 1970018 - Fix wrong prefs includefile in PerFolderDatabase. r=#thunderbird-back-end-reviewers

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/3f6eaf9a6d20
Treat unread messages as new when adding to PerFolderDatabase. r=edicharry
https://hg.mozilla.org/comm-central/rev/fca443f3bb06
Fix wrong prefs includefile in PerFolderDatabase. r=mkmelin

Comment 17

[Ben Campbell]

Attachment: Bug 1970018 - Use folder IDs instead of xpcom objects to reference folders in FolderDatabase. r=#thunderbird-back-end-reviewers

Comment 18

[Ben Campbell]

That patch removes nsIFolder (and Folder), replacing their use with integer folder IDs.
It also:

• nails down the interface documentation
• adds handling for some icky corner-cases (eg folder names containing '/').
• replaces the nsIInitiableWithFolder by adding a nsIMsgFolder.initViaPanorama() method (legacy stuff should call init(), panorama stuff calls initViaPanorama()).

It's a big patch, so I feel I need to offer some justification for it:

Firstly: I want to get away from using so much XPCOM in C++ code in favour of more ergonomic interfaces, using the concrete classes instead of abstract interfaces.
This low-level DB stuff should generally only ever be used directly by the native side.
The JS side use cases are mostly covered by liveviews (at least for querying and displaying).
Operations which manipulate the database (eg add a folder, message copying etc etc) are usually handled by other components which coordinate between database, folder, msgstore and protocol. The nsIMsgFolder is generally thing that orchestrates such operations. That side of things is mostly native and the JS view of it all should be simpler and more high level.

Secondly: It encapsulates the database better. Having XPCOM objects out there as live connections to the database spreads out the control, making it harder for the FolderDatabase to be sure about the state of things. (eg you could delete a folder, but things could still access it via an outstanding nsIFolder object). This is a big issue with the legacy code and I'd like to avoid it.

Thirdly: it makes debugging much simpler. I've already found this while converting the tests. nsCOMPtr/RefPtr and XPCOM object boundaries make debuggers extremely painful. Using folder Ids and native code that knows about concrete classes rather than XPCOM interfaces is soooo much nicer to work with. Night and day. I hate gdb, but it feels like a superpower now!

There's also code generation considerations - with an abstract (virtual) interface, even the most trivial accessor has to bounce through a vtable. When the compiler knows about concrete classes there's an awful lot of code that it can elide, optimise away or simply inline. That makes me very happy, even though it's unlikely to be a visible difference in a project this size :-)

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b6ea70cc1a21
Use folder IDs instead of xpcom objects to reference folders in FolderDatabase. r=darktrojan

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e694d5f43375
Follow up to fix build bustage, r=bustage