Closed Bug 896738 Opened 9 years ago Closed 9 years ago

Lots of TEST-UNEXPECTED-FAIL | Error console says ... in xpcshell tests

Categories

(MailNews Core :: Testing Infrastructure, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 26.0

People

(Reporter: standard8, Unassigned)

References

Details

Attachments

(14 files, 5 obsolete files)

9.58 KB, patch
jcranmer
: review+
aceman
: feedback+
Details | Diff | Splinter Review
1.06 KB, patch
standard8
: review+
Details | Diff | Splinter Review
29.73 KB, patch
mconley
: review+
Details | Diff | Splinter Review
1.03 KB, patch
ted
: review+
Details | Diff | Splinter Review
16.04 KB, patch
jcranmer
: review+
Details | Diff | Splinter Review
19.94 KB, patch
Details | Diff | Splinter Review
24.33 KB, patch
jcranmer
: review+
Details | Diff | Splinter Review
23.52 KB, patch
jcranmer
: review+
Details | Diff | Splinter Review
19.11 KB, patch
jcranmer
: review+
Details | Diff | Splinter Review
4.37 KB, patch
aceman
: review+
Details | Diff | Splinter Review
54.06 KB, patch
jcranmer
: review+
Details | Diff | Splinter Review
9.06 KB, patch
jcranmer
: review+
Details | Diff | Splinter Review
9.75 KB, patch
standard8
: review+
Details | Diff | Splinter Review
8.22 KB, patch
standard8
: review+
Details | Diff | Splinter Review
Attached patch Fix strict warnings part 1 (obsolete) — Splinter Review
This started since bug 889911 landed. What is happening, is that our tests are checking for failures logged on the console, and that bug added a lot more output, especially with respect to strict failures.

I've started working through them, although some are m-c as well which will require a separate bug. The first batch are attached for some of the ones I'm fairly sure about.
Attachment #779438 - Flags: review?(mconley)
Attachment #779438 - Flags: review?(Pidgeot18)
Depends on: 896756
Duplicate of this bug: 896909
Depends on: 896910
Depends on: 896904
Comment on attachment 779438 [details] [diff] [review]
Fix strict warnings part 1

Review of attachment 779438 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/base/search/src/nsMsgTraitService.js
@@ +119,4 @@
>    {
>      let proIndices = [];
>      let antiIndices = [];
> +    for (let id in _traits)

Please fix also the other occurence of this as said in bug 896909.

::: mailnews/test/fakeserver/imapd.js
@@ -765,5 @@
>      return this._dispatchCommand(command, args);
>    },
> -  onServerFault: function (e) {
> -    return this._tag + " BAD internal server error: " + e;
> -  },

I think you remove the wrong occurence of this function, see bug 896904. Decide if you fix it here and dupe 896904 here or drop this part from this patch.
With additional fixes for the areas that aceman highlighted.
Attachment #779438 - Attachment is obsolete: true
Attachment #779438 - Flags: review?(mconley)
Attachment #779438 - Flags: review?(Pidgeot18)
Attachment #779684 - Flags: review?(mconley)
Attachment #779684 - Flags: review?(Pidgeot18)
No longer depends on: 896904
Duplicate of this bug: 896904
Comment on attachment 779684 [details] [diff] [review]
[checked in] Fix strict warnings part 1 v2

Nice, thanks.
Attachment #779684 - Flags: feedback+
Status: NEW → ASSIGNED
Assignee: nobody → mbanner
Although I did the start patch, I'm not quite sure I'm going to have time to do all of it in a short amount of time, so others should feel free to chip in.
Assignee: mbanner → nobody
Status: ASSIGNED → NEW
Error console says [stackFrame assignment to undeclared variable gCustomSearchTermSubject] - See following stack:
https://tbpl.mozilla.org/php/getParsedLog.php?id=25606003&tree=Thunderbird-Trunk#error89

probably needs to add 'var' to http://mxr.mozilla.org/comm-central/source/mail/base/test/unit/test_viewWrapper_virtualFolderCustomTerm.js#26
Looks like it, fancy trying some patches?
Yes, I may in some hours (if my build actually succeeds after recent breakage in m-c), just posting if anybody wants to try it sooner.
For the "in strict mode code, functions may be declared only at top level or immediately within another function" error, I have found this suspicious function declaration:

http://hg.mozilla.org/comm-central/file/22a2433d1c66/mailnews/test/resources/messageGenerator.js#l1141
Comment on attachment 779684 [details] [diff] [review]
[checked in] Fix strict warnings part 1 v2

Review of attachment 779684 [details] [diff] [review]:
-----------------------------------------------------------------

Ugh, we really need to just use testing-only modules for half this stuff.

::: mailnews/test/resources/messageGenerator.js
@@ +1138,5 @@
>    for (let [name, ubfunc] in Iterator(aObj)) {
>      // the variable binding needs to get captured...
>      let realFunc = ubfunc;
> +    delete aObj[name];
> +    aObj.__defineGetter__(name, function getterFunc() {

Looking at some JS docs, this should probably be something like:
Object.defineProperty(aObj, name, {
  get: function getterFunc() {
    return realFunc.bind(this);
}});
Attachment #779684 - Flags: review?(Pidgeot18) → review+
(In reply to :aceman from comment #10)
> http://hg.mozilla.org/comm-central/file/22a2433d1c66/mailnews/test/resources/
> messageGenerator.js#l1141

Yeah and this spot seems to be handled in the patch, sorry.
Attachment #779684 - Flags: review?(mconley)
Comment on attachment 779684 [details] [diff] [review]
[checked in] Fix strict warnings part 1 v2

https://hg.mozilla.org/comm-central/rev/fc759fc258a3
Attachment #779684 - Attachment description: Fix strict warnings part 1 v2 → [checked in] Fix strict warnings part 1 v2
I'm currently looking at the testing-only modules stuff.
This fixes warning from comment 7. I could not completely verify the test as I still get many of the "function only at top level" error in this test (even after applying all the available patches).
Attachment #779919 - Flags: review?(mbanner)
This moves mailDirService.js to be a testing module, so that we no longer have to worry about it being included more than once.

There's a couple of other fixes in here, like adding a line to rules.mk so that check-one actually works (check interactive).

This probably doesn't fix any more tests yet - we need to do mailTestUtils.js for that as well - which does make it hard to know if we've improved anything or not, but for the files I've examined, it seems to work.

The returning and assigning gProfileDir, is because the module can't access do_get_profile, and if you can't change a variable value after its initial export from a module - you can only change the properties within it.
Attachment #779992 - Flags: review?(mconley)
Now without the debug print statement, and recursive remove of directories fixed.
Attachment #779992 - Attachment is obsolete: true
Attachment #779992 - Flags: review?(mconley)
Attachment #779994 - Flags: review?(mconley)
Comment on attachment 779919 [details] [diff] [review]
[checked in] declare gCustomSearchTermSubject

Looks good.
Attachment #779919 - Flags: review?(mbanner) → review+
Comment on attachment 779919 [details] [diff] [review]
[checked in] declare gCustomSearchTermSubject

https://hg.mozilla.org/comm-central/rev/e3079c599ff6
Attachment #779919 - Attachment description: declare gCustomSearchTermSubject → [checked in] declare gCustomSearchTermSubject
Comment on attachment 779994 [details] [diff] [review]
[checked in] Change mailDirService to be a testing module v2

Review of attachment 779994 [details] [diff] [review]:
-----------------------------------------------------------------

This makes sense to me, for the most part. I'm not currently in a position to test it though, so I'll assume you did. Thanks Mark!

::: mailnews/test/resources/mailDirService.js
@@ +7,4 @@
>   * For xpcshell tests, the "profile" directory will be <profile_dir>/mailtest/
>   */
>  
> +const EXPORTED_SYMBOLS = ["profileDir"];

Generally speaking, at least in my experience, exports are capitalized. If you're into it, I'd prefer ProfileDir.

@@ +22,4 @@
>  var Cr = Components.results;
>  var CC = Components.Constructor;
>  
> +var profileDir = {

let or const, not var.
Attachment #779994 - Flags: review?(mconley) → review+
Comment on attachment 779994 [details] [diff] [review]
[checked in] Change mailDirService to be a testing module v2

Yep, tested it the best I could considering we've still got many tests failing. Landed with comments addressed:

https://hg.mozilla.org/comm-central/rev/45cc1abe255a
Attachment #779994 - Attachment description: Change mailDirService to be a testing module v2 → [checked in] Change mailDirService to be a testing module v2
Attached patch WIP mailTestUtils.js switch (obsolete) — Splinter Review
This is a WIP I started for mailTestUtils.js. Combined with the previous patch it was getting a bit big, so I split it out. I'm not too keen on the export of the object that replaces gLocalIncomingServer and co, but it seems to be the only way to do it.

I guess there's going to be some automated replacements coming, however, feedback welcome.
Attachment #780010 - Flags: feedback?(mconley)
https://tbpl.mozilla.org/php/getParsedLog.php?id=25651884&tree=Thunderbird-Trunk#error82:
Error console says [stackFrame reference to undefined property msgHdr[aKeyOrValueGetter]]

This seems caused by this interesting construct:
http://mxr.mozilla.org/comm-central/source/mailnews/base/test/unit/test_nsMsgDBView.js#345:

let valueGetter = (typeof(aKeyOrValueGetter) == "string") ?
   function(msgHdr) { return msgHdr[aKeyOrValueGetter]; } : aKeyOrValueGetter;
Standard8, didn't the mailDirService.js change cause the xpcshell failures on Windows (https://tbpl.mozilla.org/php/getParsedLog.php?id=25653163&tree=Thunderbird-Trunk)?
Couldn't recursive remove directory: [Exception... "Component returned failure code: 0x8052000e (NS_ERROR_FILE_IS_LOCKED) [nsIFile.remove]"  nsresult: "0x8052000e (NS_ERROR_FILE_IS_LOCKED)"  location: "JS frame :: resource://testing-common/mailnews/mailDirService.js :: ProfileDir._recursiveRemove :: line 118"  data: no]Trying automatically
System JS : ERROR resource://testing-common/mailnews/mailDirService.js:92
                     NS_ERROR_FILE_IS_LOCKED: Component returned failure code: 0x8052000e (NS_ERROR_FILE_IS_LOCKED) [nsIFile.remove]
WARNING: nsExceptionService ignoring thread destruction after shutdown: file e:/builds/moz2_slave/tb-c-cen-w32-d-000000000000000/build/mozilla/xpcom/base/nsExceptionService.cpp, line 168
System JS : ERROR C:\talos-slave\test\build\xpcshell\head.js:247
                     TypeError: _fakeIdleService is undefined
After applying the mailTestUtils patch, the tests moved forward and uncovered these problems (when running mail/base/test). I do not convert maild.js to a module yet, I just defined the functions as variables for now. So I could find the other more interesting bugs.
Attachment #782018 - Flags: review?(mbanner)
Comment on attachment 780010 [details] [diff] [review]
WIP mailTestUtils.js switch

When I converted some tests use this module they started to work (after applying also the other fixes). So this does work.
Attachment #780010 - Flags: feedback+
Comment on attachment 780010 [details] [diff] [review]
WIP mailTestUtils.js switch

Review of attachment 780010 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, this is OK. Like I mention below, I think I'd prefer is mailTestUtils brought in a single item as opposed to a whole family of functions and items. But I guess that's just personal preference.

::: mailnews/test/resources/mailTestUtils.js
@@ +2,4 @@
>   * 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/. */
>  
> +const EXPORTED_SYMBOLS = ["loadLocalMailAccount",

I suppose this is OK...I'm usually pretty freaked out by JSM's that suddenly bring in a ton of stuff (as opposed to a single object with many properties). But I guess this is for testing only, so my resolve is lessened. :)
Attachment #780010 - Flags: feedback?(mconley) → feedback+
Comment on attachment 780010 [details] [diff] [review]
WIP mailTestUtils.js switch

It seems these imports are no longer visible to the tests including mailTestUtils.js . You would need to include them into EXPORTED_SYMBOLS.

// we would like for everyone to have fixIterator and toXPComArray
Components.utils.import("resource:///modules/iteratorUtils.jsm");
// exposes component loader's btoa impl
Components.utils.import("resource:///modules/IOUtils.js");
// JS ctypes, needed for a few native functions
Components.utils.import("resource://gre/modules/ctypes.jsm");
// Services
Components.utils.import("resource://gre/modules/Services.jsm");
// MailServices
Components.utils.import("resource:///modules/mailServices.js");
Attachment #782026 - Flags: review?(ted) → review+
Comment on attachment 782026 [details] [diff] [review]
[checked in] Declare 'msg' variable in testing/xpcshell/head.js

Thanks Ted.
Attachment #782026 - Attachment description: Declare 'msg' variable in testing/xpcshell/head.js → Declare 'msg' variable in testing/xpcshell/head.js [ready for check-in]
Keywords: checkin-needed
Whiteboard: [check in only the marked patch into mozilla-central][leave open]
Comment on attachment 782018 [details] [diff] [review]
fixes in mailnews/test/fakeserver/*

Review of attachment 782018 [details] [diff] [review]:
-----------------------------------------------------------------

I think this may be alright, but I'm not convinced about the maild changes, and I'd also like Joshua to take a look at the imap changes, so redirecting review.
Attachment #782018 - Flags: review?(mbanner) → review?(Pidgeot18)
Keywords: checkin-needed
Whiteboard: [check in only the marked patch into mozilla-central][leave open] → [leave open]
Comment on attachment 782026 [details] [diff] [review]
[checked in] Declare 'msg' variable in testing/xpcshell/head.js

https://hg.mozilla.org/integration/mozilla-inbound/rev/a18191f49ae6
Attachment #782026 - Attachment description: Declare 'msg' variable in testing/xpcshell/head.js [ready for check-in] → [on inbound] Declare 'msg' variable in testing/xpcshell/head.js
(In reply to Mark Banner (:standard8) from comment #31)
> I think this may be alright, but I'm not convinced about the maild changes,
> and I'd also like Joshua to take a look at the imap changes, so redirecting
> review.

Yes, I heard he is converting those fakeserver files to modules so then these changes are not needed. I can drop those from the patch and keep only the changes I do inside functions (undeclared variables and access to non-existent properties)
Comment on attachment 782026 [details] [diff] [review]
[checked in] Declare 'msg' variable in testing/xpcshell/head.js

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 889911
User impact if declined: None
Testing completed (on m-c, etc.): landed on inbound
Risk to taking this patch (and alternatives if risky): none
String or IDL/UUID changes made by this patch: none

This regressed because bug 889911 was block-approved as part of another bug to land on aurora. Unfortunately, that causes Thunderbird many strict errors and warnings, which we are now tidying up. This patch tides up javascript strict errors in mozilla-central.
Attachment #782026 - Flags: approval-mozilla-aurora?
I think you can just land this with a=test-only.
(In reply to :aceman from comment #33)
> (In reply to Mark Banner (:standard8) from comment #31)
> > I think this may be alright, but I'm not convinced about the maild changes,
> > and I'd also like Joshua to take a look at the imap changes, so redirecting
> > review.
> 
> Yes, I heard he is converting those fakeserver files to modules so then
> these changes are not needed. I can drop those from the patch and keep only
> the changes I do inside functions (undeclared variables and access to
> non-existent properties)

That I filed as bug 900519.
(In reply to :aceman from comment #29)
> Comment on attachment 780010 [details] [diff] [review]
> WIP mailTestUtils.js switch
> 
> It seems these imports are no longer visible to the tests including
> mailTestUtils.js . You would need to include them into EXPORTED_SYMBOLS.

We should just include them where necessary in the head files. Many of them are already imported where necessary.
Ok, I'm splitting this up into pieces, based on previous suggestions. If we export just one symbol - mailTestUtils, and have everything a sub-part of that, we can progressively move towards a state where we have something ready for a sub-module (as a side effect, we actually manage to start getting rid of some of the warnings as they land).

This first section removes the unnecessary atob and btoa suggestions, and moves loadFileToString and loadMessageToString to a module.

Although loadFileToString is now available in IOUtils, I've left it in for the one case where we need to specify the charset, that IOUtils doesn't support.
Attachment #780010 - Attachment is obsolete: true
Attachment #784429 - Flags: review?(Pidgeot18)
Comment on attachment 782018 [details] [diff] [review]
fixes in mailnews/test/fakeserver/*

Review of attachment 782018 [details] [diff] [review]:
-----------------------------------------------------------------

Drop the maild.js changes altogether.

::: mailnews/test/fakeserver/imapd.js
@@ +948,5 @@
>  
>      var func = this._kAuthSchemeStartFunction[scheme];
>      if (!func || typeof(func) != "function")
>        return "BAD I just pretended to implement AUTH " + scheme + ", but I don't";
> +    return func.call(this, (args.length > 1) ? args[1] : null);

Hmm... func.apply(this, args.slice(1)), if I remember my JS syntax correctly?

@@ +1077,5 @@
>        }
>      }
>      // check for optional list return options argument used by LIST-EXTENDED
>      // and other related RFCs
> +    if ((args.length > 2 && args[2] == "RETURN") ||

The LIST code has really gotten out of control, but I think this should work.
Attachment #782018 - Flags: review?(Pidgeot18) → review+
Moving firstMsgHdr into mailTestUtils object
Attachment #784451 - Flags: review?(Pidgeot18)
Comment on attachment 784429 [details] [diff] [review]
Load*ToString moves in mailTestUtils.js (diff -w)

Review of attachment 784429 [details] [diff] [review]:
-----------------------------------------------------------------

I was about to ask you to reindent the source code until I realized this was a diff -w :-)
Attachment #784429 - Flags: review?(Pidgeot18) → review+
This completes all the functions, apart from those relating to the local mail folder & server setup.
Attachment #784469 - Flags: review?(Pidgeot18)
Comment on attachment 784451 [details] [diff] [review]
[checked in] firstMsgHdr moves into mailTestUtils

Review of attachment 784451 [details] [diff] [review]:
-----------------------------------------------------------------

I get the feeling, looking at all these changes, that we need a msgTestUtils.loadFirstMessageToString(folder) helper.

::: mailnews/compose/test/unit/head_compose.js
@@ -171,5 @@
> -  let enumerator = folder.msgDatabase.EnumerateMessages();
> -  if (enumerator.hasMoreElements())
> -    return enumerator.getNext().QueryInterface(Ci.nsIMsgDBHdr);
> -  return null;
> -}

Huh.
Attachment #784451 - Flags: review?(Pidgeot18) → review+
Comment on attachment 784455 [details] [diff] [review]
[checked in] Move large file related functions into mailTestUtils

Review of attachment 784455 [details] [diff] [review]:
-----------------------------------------------------------------

I didn't look over this too carefully, so I'm assuming that a diff -w shows minimal changes.

::: mailnews/test/resources/mailTestUtils.js
@@ +239,4 @@
>      // Win32 type and other constants.
>      const BOOL = ctypes.int32_t;
> +    const MAX_PATH = 260;
> +  

Trailing whitespace.

@@ +410,5 @@
> +                               bytesReturned.address(), null)) {
> +            throw new Error("Unable to mark file as sparse, error " +
> +                            ctypes.winLastError);
> +          }
> +        

Trailing whitespace.
Attachment #784455 - Flags: review?(Pidgeot18) → review+
This makes mailTestUtils.js its own module, and moves the local account functions out to a separate file. Logically, I think this makes more sense, especially to help with maintenance.

Its possible I've missed adding some imports around the place, or that I've broken a couple of tests. I think with the general state of things, we'll land this and do clean up following it, as it'll be easier for more folks to look at the errors we've got outstanding.
Attachment #784518 - Flags: review?(Pidgeot18)
Thanks, carrying over r=jcranmer.
Attachment #782018 - Attachment is obsolete: true
Attachment #784531 - Flags: review+
Attachment #784469 - Flags: review?(Pidgeot18) → review+
Now with localAccountUtils.js
Attachment #784518 - Attachment is obsolete: true
Attachment #784518 - Flags: review?(Pidgeot18)
Attachment #784544 - Flags: review?(Pidgeot18)
Attachment #784544 - Attachment is patch: true
Comment on attachment 784518 [details] [diff] [review]
Make mailTestUtils into a module; move local account functions to a separate file

Review of attachment 784518 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm, can you add the --testing-modules line to mailnews/Makefile.in's xpcshell-tests run?

::: mailnews/compose/test/unit/head_compose.js
@@ -17,5 @@
>  
>  // Import the smtp server scripts
> -load("../../../fakeserver/maild.js")
> -load("../../../fakeserver/auth.js")
> -load("../../../fakeserver/smtpd.js")

This is going to bitrot hurt :-)

::: mailnews/news/test/unit/head_server_setup.js
@@ +15,4 @@
>  load("../../../fakeserver/nntpd.js");
>  
>  // Generic mailnews resource scripts
> +load("../../../resources/localAccountUtils.js");

Does NNTP actually need these?
Attachment #784518 - Attachment is obsolete: false
Comment on attachment 784544 [details] [diff] [review]
[checked in] Make mailTestUtils into a module; move local account functions to a separate file v2

Review of attachment 784544 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/test/resources/localAccountUtils.js
@@ +19,5 @@
> +{
> +  // This function is idempotent
> +  if (_localAccountInitialized)
> +    return;
> +  

Trailing whitespace :)
Attachment #784544 - Flags: review?(Pidgeot18) → review+
Comment on attachment 784430 [details] [diff] [review]
[checked in] Load*ToString moves in mailTestUtils.js (normal)

https://hg.mozilla.org/comm-central/rev/b7cfccc838e8
Attachment #784430 - Attachment description: Load*ToString moves in mailTestUtils.js (normal) → [checked in] Load*ToString moves in mailTestUtils.js (normal)
Comment on attachment 784451 [details] [diff] [review]
[checked in] firstMsgHdr moves into mailTestUtils

https://hg.mozilla.org/comm-central/rev/5a2676f7a59e
Attachment #784451 - Attachment description: firstMsgHdr moves into mailTestUtils → [checked in] firstMsgHdr moves into mailTestUtils
Comment on attachment 784455 [details] [diff] [review]
[checked in] Move large file related functions into mailTestUtils

https://hg.mozilla.org/comm-central/rev/2482c00d7ebc
Attachment #784455 - Attachment description: Move large file related functions into mailTestUtils → [checked in] Move large file related functions into mailTestUtils
Comment on attachment 784469 [details] [diff] [review]
[checked in] Moves other misc functions to mailTestUtils

https://hg.mozilla.org/comm-central/rev/b26a399e5ded
Attachment #784469 - Attachment description: Moves other misc functions to mailTestUtils → [checked in] Moves other misc functions to mailTestUtils
Attachment #784518 - Attachment is obsolete: true
Comment on attachment 784544 [details] [diff] [review]
[checked in] Make mailTestUtils into a module; move local account functions to a separate file v2

https://hg.mozilla.org/comm-central/rev/3d83e962924d
Attachment #784544 - Attachment description: Make mailTestUtils into a module; move local account functions to a separate file v2 → [checked in] Make mailTestUtils into a module; move local account functions to a separate file v2
Comment on attachment 784531 [details] [diff] [review]
[checked in] small JS-strict fixes in mailnews/test/fakeserver/* v2 [ready for checkin]

https://hg.mozilla.org/comm-central/rev/65c33aac5dfb
Attachment #784531 - Attachment description: small JS-strict fixes in mailnews/test/fakeserver/* v2 [ready for checkin] → [checked in] small JS-strict fixes in mailnews/test/fakeserver/* v2 [ready for checkin]
Landed a follow-up to fix compose tests:

https://hg.mozilla.org/comm-central/rev/7cbfcea11dc9
Attachment #782026 - Attachment description: [on inbound] Declare 'msg' variable in testing/xpcshell/head.js → [checked in] Declare 'msg' variable in testing/xpcshell/head.js
(In reply to Mark Banner (:standard8) from comment #58)
> Landed a follow-up to fix compose tests:
> 
> https://hg.mozilla.org/comm-central/rev/7cbfcea11dc9

Got mailnews xpcshell working, and found a few more bits that were missed:

https://hg.mozilla.org/comm-central/rev/f7cbf04c3704
Depends on: 900881
This fixes more issues generally that I picked up just by scanning the logs. It fixes 12 tests on my machine, and sets up a bunch of others (imap I think) ready for passing once we fix the imap pump.
Attachment #785038 - Flags: review?(Pidgeot18)
Comment on attachment 785038 [details] [diff] [review]
[checked in] Fix more issues

Review of attachment 785038 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/test/fakeserver/imapd.js
@@ +333,4 @@
>    this.uid = uid;
>    this.size = 0;
>    this.flags = new Array;
> +  for each (let flag in flags)

Change this to for (let flag of flags) ?
Attachment #785038 - Flags: review?(Pidgeot18) → review+
Comment on attachment 785038 [details] [diff] [review]
[checked in] Fix more issues

https://hg.mozilla.org/comm-central/rev/099f9cfcdec5
Attachment #785038 - Attachment description: Fix more issues → [checked in] Fix more issues
Depends on: 901048
Attachment #785157 - Flags: review?(mbanner) → review+
The imapd.js changes are basically moving a custom parser to use the main IMAP parser instead.
Attachment #785185 - Flags: review?(mbanner)
Attachment #785185 - Flags: review?(mbanner) → review+
Comment on attachment 785157 [details] [diff] [review]
[checked-in] Fix all but 2 IMAP tests on Linux

remote:   https://hg.mozilla.org/comm-central/rev/4006332617d1
Attachment #785157 - Attachment description: Fix all but 2 IMAP tests on Linux → [checked-in] Fix all but 2 IMAP tests on Linux
Comment on attachment 785185 [details] [diff] [review]
[checked in] Fix some base and MIME tests

remote:   https://hg.mozilla.org/comm-central/rev/e7c9fcd02462
Attachment #785185 - Attachment description: Fix some base and MIME tests → [checked in] Fix some base and MIME tests
Attachment #782026 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Depends on: 901537
Depends on: 901514
Depends on: 901543
Depends on: 901544
Depends on: 901678
Depends on: 901959
Depends on: 902934
Depends on: 903402
Depends on: 903946
Depends on: 904162
Apart from one possible remaining issue on bug 904162, this is fixed now. Thanks to all those that have been helping.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
Target Milestone: --- → Thunderbird 26.0
You need to log in before you can comment on or make changes to this bug.