make gloda unit tests work again

RESOLVED FIXED in Thunderbird 3.0b1

Status

P1
normal
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: asuth, Assigned: asuth)

Tracking

Trunk
Thunderbird 3.0b1
Dependency tree / graph
Bug Flags:
blocking-thunderbird3.0b1 -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [has patch][has review])

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

10 years ago
Created attachment 347110 [details] [diff] [review]
v1 make gloda unit tests work again

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)
Depends on: 463859
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)

Updated

10 years ago
Blocks: 464354
(Assignee)

Comment 2

10 years ago
Created attachment 348863 [details] [diff] [review]
v2 make gloda unit tests work again

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

10 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

10 years ago
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+
Created attachment 348946 [details] [diff] [review]
[checked in] v3 make gloda unit tests work again

This is what I checked in

http://hg.mozilla.org/comm-central/rev/ba0254757e4a
Attachment #348946 - Flags: review+
Attachment #348863 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Last Resolved: 10 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 → ---
(Assignee)

Updated

10 years ago
Depends on: 465882
(Assignee)

Updated

10 years ago
Depends on: 466083
(Assignee)

Updated

10 years ago
Priority: -- → P1
(Assignee)

Comment 7

10 years ago
Created attachment 349513 [details] [diff] [review]
v1 work on windows and choke when we log errors/warnings to log4moz

Resolve our unix-path assumptions and also abort with error if anyone logs a warning/error.
Attachment #349513 - Flags: review?(bugzilla)
(Assignee)

Updated

10 years ago
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
(Assignee)

Comment 9

10 years ago
Created attachment 349613 [details] [diff] [review]
v2 work on windows and choke when we log errors/warnings to log4moz, actually enable on windows

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

10 years ago
Keywords: checkin-needed
Whiteboard: [has patch][needs review Standard8] → [has patch][has review]
http://hg.mozilla.org/comm-central/rev/d08010138539
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.