Closed
Bug 463860
Opened 16 years ago
Closed 16 years ago
make gloda unit tests work again
Categories
(MailNews Core :: Backend, defect, P1)
MailNews Core
Backend
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3.0b1
People
(Reporter: asuth, Assigned: asuth)
References
Details
(Whiteboard: [has patch][has review])
Attachments
(2 files, 3 obsolete files)
8.64 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
10.42 KB,
patch
|
asuth
:
review+
|
Details | Diff | Splinter Review |
The gloda unit tests were broken, this makes them work again. Migrating messageGenerator.js to a more shared location and documenting it needs to be its own bug.
Attachment #347110 -
Flags: review?(bugzilla)
Comment 1•16 years ago
|
||
Comment on attachment 347110 [details] [diff] [review] v1 make gloda unit tests work again > setMessages: function(messages) { > this._messages = []; > this._totalMessageSize = 0; > > function addMessage(element) { >- this._messages.push( { fileData: readFile(element), size: -1 }); >+ // if it's a string, then it's a file-name. >+ if (typeof element == "string") >+ this._messages.push( { fileData: readFile(element), size: -1 }); >+ // otherwise it's an object as dictionary already >+ else >+ this._messages.push(element); > } Please add some documentation as to what the data format of messages can be (e.g. array of strings which are filenames, or array of objects with certain properties. >+/** >+ * Ensure the given nsIMsgFolder's database is up-to-date, calling the provided >+ * callback once the folder has been loaded. (This may be instantly or >+ * after a re-parse.) >+ */ >+function updateFolderAndNotify(aFolder, aCallback, aCallbackThis, aArgs) { Please add some documentation about what we expect these arguments to be and how they are used (e.g. its not obvious that aArgs will get passed back to the callback and not on to the nsIMsgFolder. r=me with the documentation added.
Attachment #347110 -
Flags: review?(bugzilla) → review+
Assignee | ||
Comment 2•16 years ago
|
||
requested changes have been made, but I also made an additional change to mailDirService to provide the mapping required for "resource://app/modules/gloda/BLAH.js" and so I am requesting review again. I have an inexplicable notion that it would be better to use nsIResProtocolHandler, but augmenting our existing mailDirService seems cleaner since it's more explicit and less magic. There may be a correctness issue with assuming that the gre dir is the app dir since I believe they may diverge when we move to xulrunner.
Assignee: nobody → bugmail
Attachment #347110 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #348863 -
Flags: review?(bugzilla)
Comment 3•16 years ago
|
||
not blocking b1, but we'd love to have this fixed. If you think it should block b1, let me know.
Flags: blocking-thunderbird3.0b1-
Target Milestone: --- → Thunderbird 3.0b1
Assignee | ||
Updated•16 years ago
|
Whiteboard: [has patch][needs review Standard8]
Comment 4•16 years ago
|
||
Comment on attachment 348863 [details] [diff] [review] v2 make gloda unit tests work again > return processDir; >+ } else if (prop == "resource:app") { >+ // app should return the same as gre... >+ return dirSvc.get("GreD", Ci.nsIFile); > } else if (prop == "TmpD") { > throw Components.results.NS_ERROR_FAILURE; > } else { >- dump("Wants directory: "+prop+"\n"); >+ dump("Directory request for: "+prop+" that we (mailDirService.js)"+ >+ " are not handling, leaving it to another handler.\n"); > } nit: we don't need the else parts, because of the returns. >+ * @param aFolder The nsIMSgFolder whose database you want to ensure is >+ * up-to-date. nit: case is wrong. r=me with those fixed, as I'd like to see these in the tree and running, I'm just going to address the nits and check it in in a moment.
Attachment #348863 -
Flags: review?(bugzilla) → review+
Comment 5•16 years ago
|
||
This is what I checked in http://hg.mozilla.org/comm-central/rev/ba0254757e4a
Attachment #348946 -
Flags: review+
Updated•16 years ago
|
Attachment #348863 -
Attachment is obsolete: true
Updated•16 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Flags: in-testsuite+
Whiteboard: [has patch][needs review Standard8]
Comment 6•16 years ago
|
||
I have just committed http://hg.mozilla.org/comm-central/rev/f0e1e3014bfd which disables the gloda unit tests on Windows - we were getting a test failure, here's the relevant section of the log: TEST-PASS | ../../../../mozilla/_tests/xpcshell-simple/test_mailnewsglobaldb/unit/test_index_messages.js | all tests passed TEST-UNEXPECTED-FAIL | ../../../../mozilla/_tests/xpcshell-simple/test_mailnewsglobaldb/unit/test_index_messages_in_bulk.js | test failed, see log ../../../../mozilla/_tests/xpcshell-simple/test_mailnewsglobaldb/unit/test_index_messages_in_bulk.js.log: >>>>>>> Directory request for: SysD that we (mailDirService.js) are not handling, leaving it to another handler. *** test pending Directory request for: MailD that we (mailDirService.js) are not handling, leaving it to another handler. Directory request for: MFCaF that we (mailDirService.js) are not handling, leaving it to another handler. Directory request for: DefRt that we (mailDirService.js) are not handling, leaving it to another handler. *** test pending ====== Test function: test_index_a_bunch [Exception... "Component returned failure code: 0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH) [nsILocalFile.appendRelativePath]" nsresult: "0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH)" location: "JS frame :: d:/buildbot/trunk-win32/build/mozilla/../mailnews/db/gloda/test/resources/messageGenerator.js :: writeMessagesToMbox :: line 186" data: no] *** FAIL *** 2008-11-19 02:28:46 gloda.NS INFO Logging Initialized 2008-11-19 02:28:46 gloda.datastore DEBUG Beginning datastore initialization. 2008-11-19 02:28:46 gloda.datastore DEBUG Creating database because it does't exist. 2008-11-19 02:28:46 gloda.datastore INFO Create fulltext: CREATE VIRTUAL TABLE conversationsText USING fts3(tokenize porter, subject TEXT) 2008-11-19 02:28:46 gloda.datastore INFO Create fulltext: CREATE VIRTUAL TABLE messagesText USING fts3(tokenize porter, subject TEXT, body TEXT, attachmentNames TEXT) 2008-11-19 02:28:46 gloda.datastore DEBUG Initializing folder mappings. 2008-11-19 02:28:46 gloda.datastore DEBUG Populating managed id counters. 2008-11-19 02:28:46 gloda.datastore DEBUG Completed datastore initialization. 2008-11-19 02:28:46 gloda.NS INFO Defining noun: bool 2008-11-19 02:28:46 gloda.NS INFO Defining noun: number 2008-11-19 02:28:46 gloda.NS INFO Defining noun: string 2008-11-19 02:28:46 gloda.NS INFO Defining noun: date 2008-11-19 02:28:46 gloda.NS INFO Defining noun: fulltext 2008-11-19 02:28:46 gloda.NS INFO Defining noun: folder 2008-11-19 02:28:46 gloda.NS INFO Defining noun: conversation 2008-11-19 02:28:46 gloda.NS INFO Defining noun: message 2008-11-19 02:28:46 gloda.NS INFO Defining noun: contact 2008-11-19 02:28:46 gloda.NS INFO Defining noun: identity 2008-11-19 02:28:46 gloda.NS INFO Defining noun: parameterized-identity 2008-11-19 02:28:46 gloda.datastore INFO loading all attribute defs 2008-11-19 02:28:46 gloda.datastore INFO done loading all attribute defs 2008-11-19 02:28:46 gloda.everybody INFO ... loading resource://app/modules/gloda/fundattr.js 2008-11-19 02:28:46 gloda.noun.mimetype DEBUG initializing table definition 2008-11-19 02:28:46 gloda.noun.mimetype DEBUG loading MIME types 2008-11-19 02:28:46 gloda.NS INFO Defining noun: mime-type 2008-11-19 02:28:46 gloda.everybody INFO +++ loaded resource://app/modules/gloda/fundattr.js 2008-11-19 02:28:46 gloda.everybody INFO +++ inited resource://app/modules/gloda/fundattr.js 2008-11-19 02:28:46 gloda.everybody INFO ... loading resource://app/modules/gloda/explattr.js 2008-11-19 02:28:46 gloda.NS INFO Defining noun: tag 2008-11-19 02:28:46 gloda.everybody INFO +++ loaded resource://app/modules/gloda/explattr.js 2008-11-19 02:28:46 gloda.everybody INFO +++ inited resource://app/modules/gloda/explattr.js 2008-11-19 02:28:46 gloda.everybody INFO ... loading resource://app/modules/gloda/noun_tag.js 2008-11-19 02:28:46 gloda.everybody INFO +++ loaded resource://app/modules/gloda/noun_tag.js 2008-11-19 02:28:46 gloda.everybody INFO ... loading resource://app/modules/gloda/noun_freetag.js 2008-11-19 02:28:46 gloda.NS INFO Defining noun: freetag 2008-11-19 02:28:46 gloda.everybody INFO +++ loaded resource://app/modules/gloda/noun_freetag.js 2008-11-19 02:28:46 gloda.everybody INFO ... loading resource://app/modules/gloda/noun_mimetype.js 2008-11-19 02:28:46 gloda.everybody INFO +++ loaded resource://app/modules/gloda/noun_mimetype.js 2008-11-19 02:28:46 gloda.everybody INFO ... loading resource://app/modules/gloda/index_ab.js 2008-11-19 02:28:46 gloda.indexer INFO Event-Driven Indexing is now true 2008-11-19 02:28:46 gloda.indexer INFO Registering indexer: ab_indexer 2008-11-19 02:28:46 gloda.everybody INFO +++ loaded resource://app/modules/gloda/index_ab.js 2008-11-19 02:28:46 gloda.everybody INFO +++ inited resource://app/modules/gloda/index_ab.js <<<<<<< TEST-PASS | ../../../../mozilla/_tests/xpcshell-simple/test_mailnewsglobaldb/unit/test_query_messages.js | all tests passed
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Updated•16 years ago
|
Priority: -- → P1
Assignee | ||
Comment 7•16 years ago
|
||
Resolve our unix-path assumptions and also abort with error if anyone logs a warning/error.
Attachment #349513 -
Flags: review?(bugzilla)
Assignee | ||
Updated•16 years ago
|
Whiteboard: [has patch][needs review Standard8]
Updated•16 years ago
|
Attachment #349513 -
Flags: review?(bugzilla) → review+
Comment 8•16 years ago
|
||
Comment on attachment 349513 [details] [diff] [review] v1 work on windows and choke when we log errors/warnings to log4moz This looks fine. Will you be doing a separate patch to actually enable on windows? If you've just forgotten it in this patch (and windows works), then r=me to remove the two additional lines from the relevant Makefile.in
Assignee | ||
Comment 9•16 years ago
|
||
only change is to remove the windows-only lines pointed out by Standard8. carrying forward r=Standard8
Attachment #349513 -
Attachment is obsolete: true
Attachment #349613 -
Flags: review+
Assignee | ||
Updated•16 years ago
|
Keywords: checkin-needed
Whiteboard: [has patch][needs review Standard8] → [has patch][has review]
Comment 10•16 years ago
|
||
http://hg.mozilla.org/comm-central/rev/d08010138539
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•