[SeaMonkey 2.0] xpcshell-tests: test_autoconfigUtils.js fails (and leaks) after bug 538568 landing

VERIFIED FIXED in Thunderbird 3.1b1

Status

MailNews Core
Account Manager
--
major
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

(Depends on: 1 bug, Blocks: 1 bug, {fixed-seamonkey2.0.3, regression})

Trunk
Thunderbird 3.1b1
fixed-seamonkey2.0.3, regression
Dependency tree / graph
Bug Flags:
in-testsuite +

Thunderbird Tracking Flags

(thunderbird3.0 .2-fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

7 years ago
All 3 platforms:
{
TEST-UNEXPECTED-FAIL | e:\builds\slave\comm-1.9.1-win32-unittest\build\objdir\mozilla\_tests\xpcshell\test_mailnewsbase\unit\test_autoconfigUtils.js | test failed (with xpcshell return code: 3), see following log:

uncaught exception: ContentLength not available (not a local URL?)

== BloatView: ALL (cumulative) LEAK STATISTICS
}
(Assignee)

Comment 1

7 years ago
Blake, Mark, any hint?
Summary: [SeaMonkey 2.0] test_autoconfigUtils.js fails (and leaks) after bug 535568 landing → [SeaMonkey 2.0] test_autoconfigUtils.js fails (and leaks) after bug 538568 landing
I took a look, but can't figure out why it would do that for SM, but not TB.

(To be fair, I haven't built SeaMonkey yet, and don't really have the time to until mid-February.)

Does it stop failing and/or stop leaking if you comment out the call to "test_copying_readFromXML();" on line 355 of mailnews/base/test/unit/test_autoconfigUtils.js ?

Thanks,
Blake.
(Assignee)

Comment 3

7 years ago
(In reply to comment #2)

> I took a look, but can't figure out why it would do that for SM, but not TB.

I would guess because SM does not have the "accountcreation" feature (yet)?

> (To be fair, I haven't built SeaMonkey yet, and don't really have the time to
> until mid-February.)

You can use a packaged tests SM tinderbox build :-)

> Does it stop failing and/or stop leaking if you comment out the call to
> "test_copying_readFromXML();" on line 355 of
> mailnews/base/test/unit/test_autoconfigUtils.js ?

Same error.
The error is caused by any of the 4 |loader.loadSubScript("chrome://messenger/content/accountcreation/*.js"|.
Summary: [SeaMonkey 2.0] test_autoconfigUtils.js fails (and leaks) after bug 538568 landing → [SeaMonkey 2.0] xpcshell-tests: test_autoconfigUtils.js fails (and leaks) after bug 538568 landing
Okay, so if you're not using the autoconfig, does it make any sense to run this test in the first place?

And if not, can we remove it only when SeaMonkey runs the test suite?
(Assignee)

Comment 5

7 years ago
(In reply to comment #4)

I don't know much about this very feature, but:
it looks like it was meant to live in MNCore.
I think the usual way is to check whether it's enabled or not when trying to test it.
Sounds good.  If it's still not fixed by the time I get some free time, I'll work on that.

Thanks,
Blake.
Well according to mailnews/jar.mn there's an ifdef in there for MOZ_THUNDERBIRD for those js files. So the test just isn't going to be able to load them.

Dunno why, I expect the reason is lost in the midst of time. Doesn't matter anyway, we just need to take account of it in the test somehow.

Serge: maybe try adding a try/catch and then something in run_test to return early?
(Assignee)

Comment 8

7 years ago
Created attachment 422273 [details] [diff] [review]
(Av1) Rough handling for SeaMonkey case, Let the log be somewhat more explicit.

Per comment 7. I'm not sure if we can do any better ftb :-|
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #422273 - Flags: review?(bugzilla)
(Assignee)

Updated

7 years ago
Depends on: 468196
Comment on attachment 422273 [details] [diff] [review]
(Av1) Rough handling for SeaMonkey case, Let the log be somewhat more explicit.

Whitespace... and why the extra do_check's that really "do" nothing?

[If mark r+'es ignore my r- of course]
Attachment #422273 - Flags: review-
(In reply to comment #9)
> (From update of attachment 422273 [details] [diff] [review])
> Whitespace... 

By this I mean indents where you braced should exist.
(Assignee)

Comment 11

7 years ago
(In reply to comment #9)

> Whitespace... 

Sure, I could re-indent both blocks if you want: it just happens that some times (as an exception) we don't...

> and why the extra do_check's that really "do" nothing?

To get a trace/count in the log: see bug 468196.
(In reply to comment #11)
> (In reply to comment #9)
> > and why the extra do_check's that really "do" nothing?
> 
> To get a trace/count in the log: see bug 468196.

If you mean because of: // These will trigger the 'deprecated' (harness) warning.

I'm still not even sure on why your adding them here. Wouldn't the other parts of the test still generate something useful relating to that?

If you mean something else, can you be a bit more specific.

Besides I don't see anything here that depends on 468196 or vice-versa (why not leave any "other" features of this to a new patch. Properly explained in its own bug?

Comment 13

7 years ago
(In reply to comment #7)
> Well according to mailnews/jar.mn there's an ifdef in there for MOZ_THUNDERBIRD
> for those js files.
> Dunno why, I expect the reason is lost in the midst of time.

We decided that the account autoconfig is a bit too experimental for SeaMonkey 2.0, which already had enough other stuff we needed to get finished up and working well enough. We should revisit and possibly change that for 2.1, though.

Just as a note.
(Assignee)

Comment 14

7 years ago
(In reply to comment #12)

This test currently logs nothing about its checks except in case of failure: |do_throw(aWhy);|.
Adding logs let know what actually happened ... and be sure that it did not just skip everything for example: that's how reftests and mochitests work.

Bug 468196 dependency is just a reminder that this test badly needs more detailed messages: 400 times the same log is the best that can be done ftb, but is hardly user friendly.

I'm doing it all at once because the test author doesn't care about the perma-orange and the log changes are trivial.
(Assignee)

Comment 15

7 years ago
(In reply to comment #13)
> We should revisit and possibly change that for 2.1, though.

Ftr, this is bug 507841.
Comment on attachment 422273 [details] [diff] [review]
(Av1) Rough handling for SeaMonkey case, Let the log be somewhat more explicit.

>+
>+try {
>+
> var loader = Components.classes["@mozilla.org/moz/jssubscript-loader;1"]
>                        .getService(Components.interfaces.mozIJSSubScriptLoader);
> loader.loadSubScript("chrome://messenger/content/accountcreation/util.js",
>                      xmlReader);
> loader.loadSubScript("chrome://messenger/content/accountcreation/accountConfig.js",
>                      xmlReader);
> loader.loadSubScript("chrome://messenger/content/accountcreation/sanitizeDatatypes.js",
>                      xmlReader);
> loader.loadSubScript("chrome://messenger/content/accountcreation/readFromXML.js",
>                      xmlReader);
>

This needs correct indentation.
 
>+} catch (ex) {
>+  // The "accountcreation" files are not available in SeaMonkey (yet).
>+  do_check_false(false);

One issue per bug please. If we're doing this for bug 468196 then a) this bad check needs a clear explanation of why it is being added (IMHO there should be some sort of "info" option if we need a check such as this that badly) and b) we need to have proper checkin comment for blame.

> function assert(aBeTrue, aWhy)
> {
>-  if (!aBeTrue)
>+  if (aBeTrue)
>+    do_check_true(true);
>+  else
>     do_throw(aWhy);

Ditto, we don't need this change to fix this issue.

>+
>+  if (xmlReader) {
>+
>   test_copying_readFromXML();
> 
>+  }

There's no excuse not to indent that.
Attachment #422273 - Flags: review?(bugzilla) → review-
(Assignee)

Comment 17

7 years ago
Created attachment 423558 [details] [diff] [review]
(Av2) Rough handling that "accountcreation" files are not available in SeaMonkey (yet)
[Checkin: See comment 19 & 20]

Av1, with comment 16 suggestion(s).
Attachment #422273 - Attachment is obsolete: true
Attachment #423558 - Flags: review?(bugzilla)
Attachment #423558 - Flags: review?(bugzilla) → review+
Comment on attachment 423558 [details] [diff] [review]
(Av2) Rough handling that "accountcreation" files are not available in SeaMonkey (yet)
[Checkin: See comment 19 & 20]

>+  if (xmlReader) {
>+    test_copying_readFromXML();
>+  }

nit: (sorry only just realised this) no brackets necessary here.

r=Standard8 with that fixed.
Attachment #423558 - Flags: approval-thunderbird3.0.2+
(Assignee)

Comment 19

7 years ago
Comment on attachment 423558 [details] [diff] [review]
(Av2) Rough handling that "accountcreation" files are not available in SeaMonkey (yet)
[Checkin: See comment 19 & 20]


http://hg.mozilla.org/comm-central/rev/3a0de036bf83
Av2, with comment 18 suggestion(s).
Attachment #423558 - Attachment description: (Av2) Rough handling that "accountcreation" files are not available in SeaMonkey (yet) → (Av2) Rough handling that "accountcreation" files are not available in SeaMonkey (yet) [Checkin: See comment 19]
(Assignee)

Comment 20

7 years ago
Comment on attachment 423558 [details] [diff] [review]
(Av2) Rough handling that "accountcreation" files are not available in SeaMonkey (yet)
[Checkin: See comment 19 & 20]


http://hg.mozilla.org/releases/comm-1.9.1/rev/2ce707a354cd
Attachment #423558 - Attachment description: (Av2) Rough handling that "accountcreation" files are not available in SeaMonkey (yet) [Checkin: See comment 19] → (Av2) Rough handling that "accountcreation" files are not available in SeaMonkey (yet) [Checkin: See comment 19 & 20]
(Assignee)

Updated

7 years ago
Severity: normal → major
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
status-thunderbird3.0: --- → .2-fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.1b1
Version: 1.9.1 Branch → Trunk
(Assignee)

Comment 21

7 years ago
V.Fixed, per SeaMonkey 2.0 tinderboxes.
Status: RESOLVED → VERIFIED

Updated

7 years ago
Keywords: fixed-seamonkey2.0.3
Verifying as fixed for 3.0.2 based on the fact that these are test-only patches and the last 5 runs from MozMill/unit tests have been green with no failures relating to these patches.
Keywords: verified-thunderbird3.0
You need to log in before you can comment on or make changes to this bug.