Closed Bug 463860 Opened 16 years ago Closed 16 years ago

make gloda unit tests work again

Categories

(MailNews Core :: Backend, defect, P1)

defect

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)

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 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+
Blocks: 464354
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)
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
Whiteboard: [has patch][needs review Standard8]
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+
Attachment #348863 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Flags: in-testsuite+
Whiteboard: [has patch][needs review Standard8]
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 → ---
Depends on: 465882
Depends on: 466083
Priority: -- → P1
Resolve our unix-path assumptions and also abort with error if anyone logs a warning/error.
Attachment #349513 - Flags: review?(bugzilla)
Whiteboard: [has patch][needs review Standard8]
Attachment #349513 - Flags: review?(bugzilla) → review+
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
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+
Keywords: checkin-needed
Whiteboard: [has patch][needs review Standard8] → [has patch][has review]
http://hg.mozilla.org/comm-central/rev/d08010138539
Status: REOPENED → RESOLVED
Closed: 16 years ago16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: