Closed Bug 1457087 Opened 6 years ago Closed 6 years ago

Massive Calendar Xpcshell test failure 2018-04-26: all tests failing

Categories

(Calendar :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jorgk-bmo, Assigned: Fallen)

References

Details

(Whiteboard: [Thunderbird-testfailure: X all][Thunderbird-disabled-test])

Attachments

(3 files, 1 obsolete file)

comm/calendar/test/unit/test_bug494140.js
comm/calendar/test/unit/test_alarmutils.js 
comm/calendar/test/unit/test_alarmservice.js
comm/calendar/test/unit/test_bug343792.js
comm/calendar/test/unit/test_bug356207.js
comm/calendar/test/unit/test_ics.js
comm/calendar/test/unit/test_bug759324.js
comm/calendar/test/unit/test_calmgr.js
comm/calendar/test/unit/test_datetime.js
comm/calendar/test/unit/test_providers.js
Keywords: leave-open
Attached patch 1457087-switch-off-tests.patch (obsolete) β€” β€” Splinter Review
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/3b668ee2e9b2
temporarily switch off failing calendar tests. rs=bustage-fix DONTBUILD
I've switched these tests off for now to allow for effective sheriffing.

Let's try to find the cause:
M-C last good: 7f6a582f00bfb5d0acb8d8bf7f8c79ca37
M-C first bad: b62ad926cf2a2d5759222f4e9b40c9e3bd

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7f6a582f00bfb5d0acb8d8bf7f8c79ca37&tochange=b62ad926cf2a2d5759222f4e9b40c9e3bd

Could be related to bug 1456677 (although that's about blocklists) or bug 1456035.

The log
https://taskcluster-artifacts.net/T9UhV8PFQhajq-D8NZeoPw/0/public/logs/live_backing.log
isn't really so conclusive:
JavaScript strict warning: /Users/cltbld/tasks/task_1524727248/build/tests/xpcshell/tests/comm/calendar/test/unit/test_ics.js, line 107: ReferenceError: reference to undefined property "startDate"
JavaScript strict warning: resource://calendar/modules/utils/calItemUtils.jsm, line 283: ReferenceError: reference to undefined property "completedDate"
JavaScript strict warning: resource://gre/modules/XPCOMUtils.jsm, line 277: ReferenceError: reference to undefined property 1
(In reply to Jorg K (GMT+1) from bug 1456677 comment #10)
> Gijs and Kris, do we need to port anything else here? In this merge we get
> 10 Calendar Xpcshell failures, bug 1457087, but nothing that looks like
> blocklist related.:gijs

So I'm a little bit at sea because you didn't link to the orange you meant, and there were a lot of pushes on the 23rd, and I don't understand how comm-central works with m-c tip. So take the following with appropriate amounts of salt etc.

I could be wrong, but I think the calendar issues are related to other things in those pushes. Specifically, there is eslint orange on the push with the calendar failures:

https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=069aa51eefe67ae85a0768a9eaf570b840126b99&selectedJob=175674634

TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/modules/utils/calACLUtils.jsm:8:1 | 'cal' is defined but never used. Allowed unused vars must match /EXPORTED_SYMBOLS/. (no-unused-vars)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/modules/utils/calXMLUtils.jsm:7:1 | 'cal' is defined but never used. Allowed unused vars must match /EXPORTED_SYMBOLS/. (no-unused-vars)

which just looks like one of the things there might have broken calendar, maybe the bug 1440587 stuff (which is a bit scary because it looks like that was uplifted to beta). Maybe :Fallen can take a look.

There's some X4 xpcshell orange on seemingly-related tests (telemetry environment etc.) but that seems to predate Kris' patches (ie I see it on pushes on April 13th). I don't know how old that is; if it's related to earlier blocklist changes please let me know.
Flags: needinfo?(philipp)
Thanks, Gijs. Yes, this is the push, and the oranges are here for example:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=069aa51eefe67ae85a0768a9eaf570b840126b99&selectedJob=175678705

I don't think the eslint failures are significant, but I've already filed a bug from them this morning: Bug 1457064. I find it a little disconcerting that those have popped up since apart from making string changes, nothing changed in Calendar in that push. String changes:
https://hg.mozilla.org/comm-central/rev/aaebcb45064cf4d54cad75594ee85cc8b4ede130
Bug 1440587 did a lot of stuff, but this was just a trivial follow-up.

The X4 failures are reported in bug 1448831, apparently some Windows locking issue that only happens on C-C and not M-C. No time to look at it further.

I think the Calendar guys need to investigate here.
Flags: needinfo?(mschroeder)
Flags: needinfo?(makemyday)
Flags: needinfo?(Mozilla)
Whiteboard: [Thunderbird-testfailure: X all][Thunderbird-disabled-test]
Tooru-san, you have a good eye for regressions, can you see anything in this range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7f6a582f00bfb5d0acb8d8bf7f8c79ca37&tochange=b62ad926cf2a2d5759222f4e9b40c9e3bd
Flags: needinfo?(arai.unmht)
(In reply to Jorg K (GMT+1) from comment #5)
> I don't think the eslint failures are significant, but I've already filed a
> bug from them this morning: Bug 1457064. I find it a little disconcerting
> that those have popped up since apart from making string changes, nothing
> changed in Calendar in that push. String changes:
> https://hg.mozilla.org/comm-central/rev/
> aaebcb45064cf4d54cad75594ee85cc8b4ede130

Maybe there were eslint changes on m-c?
Sadly, after this push
https://hg.mozilla.org/comm-central/rev/3b668ee2e9b2
temporarily switch off failing calendar tests. rs=bustage-fix DONTBUILD
there are till may more test failures:
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:comm/calendar/test/unit/test_timezone.js | xpcshell return code: 0
TEST-UNEXPECTED-TIMEOUT | xpcshell-icaljs.ini:comm/calendar/test/unit/test_freebusy.js | Test timed out
TEST-UNEXPECTED-TIMEOUT | xpcshell-icaljs.ini:comm/calendar/test/unit/test_freebusy_service.js | Test timed out
TEST-UNEXPECTED-TIMEOUT | xpcshell-icaljs.ini:comm/calendar/test/unit/test_deleted_items.js | Test timed out
TEST-UNEXPECTED-TIMEOUT | xpcshell-icaljs.ini:comm/calendar/test/unit/test_ltninvitationutils.js | Test timed out
TEST-UNEXPECTED-TIMEOUT | xpcshell-icaljs.ini:comm/calendar/test/unit/test_datetimeformatter.js | Test timed out
TEST-UNEXPECTED-TIMEOUT | xpcshell-icaljs.ini:comm/calendar/test/unit/test_items.js | Test timed out
TEST-UNEXPECTED-TIMEOUT | xpcshell-icaljs.ini:comm/calendar/test/unit/test_ics_service.js | Test timed out
TEST-UNEXPECTED-TIMEOUT | xpcshell-icaljs.ini:comm/calendar/test/unit/test_ics_parser.js | Test timed out
TEST-UNEXPECTED-TIMEOUT | xpcshell-icaljs.ini:comm/calendar/test/unit/test_recur.js | Test timed out
TEST-UNEXPECTED-TIMEOUT | xpcshell-icaljs.ini:comm/calendar/test/unit/test_rfc3339_parser.js | Test timed out
TEST-UNEXPECTED-TIMEOUT | xpcshell-icaljs.ini:comm/calendar/test/unit/test_storage.js | Test timed out
TEST-UNEXPECTED-TIMEOUT | xpcshell-icaljs.ini:comm/calendar/test/unit/test_timezone_definition.js | Test timed out
TEST-UNEXPECTED-TIMEOUT | xpcshell-icaljs.ini:comm/calendar/test/unit/test_utils.js | Test timed out

Those timeouts make the Xpcshell tests fail after 90 minutes, instead of normally taking 10 minutes. Not a good place to be.

Noteworthy:
"CONSOLE_MESSAGE: (error) [JavaScript Error: "TypeError: service[method] is not a function" {file: "resource://calendar/modules/calUtils.jsm -> file:///builds/worker/workspace/build/application/thunderbird/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/calendar-js/calStartupService.js" line: 21}]"
Or maybe all the tests will fail, it's just a matter of luck how many get run before the timeout hits. Different platforms show different failures, so that would confirm that.

I will disable Calendar completely since a) it doesn't work b) Mozimill is disabled already c) the Xpcshell test failures don't allow efficient development, try run and sheriffing.
Summary: Massive Calendar Xpcshell test failure 2018-04-26: 10 failing tests → Massive Calendar Xpcshell test failure 2018-04-26: all tests failing
Attachment #8971173 - Attachment is obsolete: true
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/21722119d184
Backed out changeset 3b668ee2e9b2 a=backout
https://hg.mozilla.org/comm-central/rev/8744c2251b63
Disable building Calendar/Lightning. rs=bustage-fix
Running a test locally, I see:

 0:01.47 INFO "CONSOLE_MESSAGE: (error) [JavaScript Error: "TypeError: service[method] is not a function" {file: "resource://calendar/modules/calUtils.jsm -> file:///c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/dist/bin/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/calendar-js/calStartupService.js" line: 21}]"

And it's hung there.
The first failure-messages don't point in that direction, but with the Timeouts, I wonder if this can be related to Bug 1449487?
Flags: needinfo?(Mozilla)
Well, yes, Lightning as a non-bootstrapped add-on isn't working and we disabled Mozmill. Somehow Xpcshell tests still ran, and I was waiting for the day they would fail. This day has come apparently, but I'd still like to understand what caused the failure now in this M-C merge of this morning:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7f6a582f00bfb5d0acb8d8bf7f8c79ca37&tochange=b62ad926cf2a2d5759222f4e9b40c9e3bd

BTW, the build failure covered in the try runs was fixed as a follow-up in bug 1439469. So the tree is green again, but without Lightning.
All four try runs didn't show the failure. So this brings the regression range down to:
Last Good: c6fea87acb49 - c6fea87acb491d99fdae69335dbb902c9b
First Bad: b62ad926cf2a - b62ad926cf2a2d5759222f4e9b40c9e3bd
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c6fea87acb49&tochange=b62ad926cf2a
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c6fea87acb491d99fdae69335dbb902c9b&tochange=b62ad926cf2a2d5759222f4e9b40c9e3bd
For some reason, the above link don't work for some unknown reason :-( So I'm copying the range here:

b62ad926cf2a	Andreea Pavel β€” Merge mozilla-inbound to mozilla-central. a=merge
822e81707327	Jeff Walden β€” Bug 1451248. r=jorendorff, r=bz
3010f9088dc7	Jeff Walden β€” Bug 1456296 - Move IdentifierName parsing into a separate function from TSS::getTokenInternal to simplify some control flow. r=arai
595101eb4739	Brian Birtles β€” Bug 1454123 - Wait a moment after moving toolbox to a window before triggering its menu; r=bgrins
3e771c2f48b0	Kris Maglione β€” Bug 1456035: Follow-up: Fix wrapper error in plain mochitest. r=bustage
63241afeab20	Kris Maglione β€” Bug 1456035: Follow-up: Delete redundant test_extensionURL.html, which fails with wrapper errors. r=me f=aswan CLOSED TREE
bd589df323a9	Shane Caraveo β€” Bug 1455755 Move browserSettings.proxyConfig to proxy.settings, r=aswan, mstrimer
35fe51311264	Kris Maglione β€” Bug 1456677: Make the blocklist service a JSM, with an XPCOM service stub. r=Gijs
2add2de30c22	Kris Maglione β€” Bug 1456035: Part 3.1 - Add temporary fallback XPCOMUtils.generateQI implementation for Android hostutils. r=me
affc06c38acd	Kris Maglione β€” Bug 1456035: Part 3 - Replace XPCOMUtils.generateQI with a stub for ChromeUtils.generateQI. r=mccr8
0102a61e38ae	Kris Maglione β€” Bug 1456035: Part 2 - Add fast path for XPCWrappedJS QueryInterface with native helper. r=mccr8
a442c0b9dbbe	Kris Maglione β€” Bug 1456035: Part 1 - Add helper to generate native QueryInterface callbacks. r=bz
c6fea87acb49	arthur.iakab β€” Merge mozilla-central to inbound on a CLOSED TREE

Kris, what has changed in that range so that our non-bootstrapped Calendar add-on, which was limping along since non-bootstrapped add-ons were (incompletely) disabled, now also doesn't work in Xpcshell tests any more?
Flags: needinfo?(mschroeder) → needinfo?(kmaglione+bmo)
I've done another (failed) try at 3e771c2f48b0, so that confirms bug 1456035.

Kris, this doesn't look like add-on work, so why would that affect the Xpshell tests of the (half-dead) non-bootstrapped Calendar add-on? The major issue seems to be comment #12. Or perhaps Boris or Andrew have an idea.
Flags: needinfo?(continuation)
Flags: needinfo?(bzbarsky)
If I had to guess, I'd say it's because XPCOMUtils.generateQI no longer special cases nsIClassInfo to return this.classInfo (which is unused in m-c and more or less going away).

Not sure what you mean by "incompletely disabled". The code for loading non-bootstrapped extensions is very much gone.
Flags: needinfo?(kmaglione+bmo)
Thanks for the guess :-)

(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #20)
> If I had to guess, I'd say it's because XPCOMUtils.generateQI no longer
> special cases nsIClassInfo to return this.classInfo (which is unused in m-c
> and more or less going away).
That doesn't mean anything to me, but I hope the JS experts from the Calendar team will understand what that's about.

> Not sure what you mean by "incompletely disabled". The code for loading
> non-bootstrapped extensions is very much gone.
Well, no offence intended. I'll explain. Although non-bootstrapped add-ons are no longer supported since bug 1447831 and bug 1448221, we were still building and packaging the non-bootstrapped Lightning/Calendar add-on and were running Xpcshell tests with it until they all started to fail due to bug 1456035. Is it therefore completely unreasonable to call that "incompletely disabled".
Flags: needinfo?(continuation)
Flags: needinfo?(bzbarsky)
Flags: needinfo?(arai.unmht)
(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #20)
> If I had to guess, I'd say it's because XPCOMUtils.generateQI no longer
> special cases nsIClassInfo to return this.classInfo (which is unused in m-c
> and more or less going away).
Are you saying nsIClassInfo is on its way out, or just the this.classInfo special case? I tried doing the conversion here, but I have a few methods that return calIBaseClass which can be a calIThingA or calIThingB. With correct class info I don't need to specifically QI the result to e.g. calIThingA, but can just use it and xpcom/xpconnect will do the right thing.
Flags: needinfo?(philipp)
Flags: needinfo?(makemyday)
Flags: needinfo?(kmaglione+bmo)
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attached patch Fix - v1 β€” β€” Splinter Review
This fixes the issues by using a custom generateQI (almost) everywhere where more than one interface is supported. Long term we probably just want to get rid of all the xpcom classes in calendar, but that isn't exactly a small patch.

Kris, despite patch I'd still be interested to hear what the future of nsIClassInfo is. It seems there are some mechanics in the platform that use nsIClassInfo to give an xpcom wrapped object the classes it needs, and I'd like to avoid having to sprinkle .QueryInterface(Ci.calIObvious) everywhere.
Attachment #8973160 - Flags: review?(makemyday)
Did you run the tests locally? I can't see any try run.
Comment on attachment 8973160 [details] [diff] [review]
Fix - v1

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

Thanks for bringing the xpcshell tests back! r+ with the below comments considered.

Xpcshell test are running locally for me with this patch applied on top of the patch 1452120, but a try push wouldn't hurt.

::: calendar/base/backend/icaljs/calRecurrenceRule.js
@@ +12,5 @@
>  }
>  
> +var calRecurrenceRuleInterfaces = [
> +    Components.interfaces.calIRecurrenceRule,
> +    Components.interfaces.calIRecurrenceItem

calIReccurrenceItem wasn't included before. Why is that needed now?

Apart from that, we should convert Components.interfaces to Ci at all places we touch it (you changed this at some places already). But since we have a separate bug for it, I spare further comments on that in this review.

::: calendar/base/modules/calUtils.jsm
@@ +189,5 @@
> +        if (aInterfaces.length == 1) {
> +            cal.WARN("When generating QI for one interface, please use ChromeUtils.generateQI", cal.STACK(10));
> +            return ChromeUtils.generateQI(aInterfaces);
> +        } else {
> +            /* Note that Ci[Ci.x] == Ci.x for all x */

What's this comment for?

@@ +192,5 @@
> +        } else {
> +            /* Note that Ci[Ci.x] == Ci.x for all x */
> +            let names = [];
> +            if (aInterfaces) {
> +                for (let i = 0; i < aInterfaces.length; i++) {

Can you switch to for (let iface of interfaces) here?
Attachment #8973160 - Flags: review?(makemyday) → review+
(In reply to Philipp Kewisch [:Fallen]  from comment #23)
> Kris, despite patch I'd still be interested to hear what the future of
> nsIClassInfo is. It seems there are some mechanics in the platform that use
> nsIClassInfo to give an xpcom wrapped object the classes it needs, and I'd
> like to avoid having to sprinkle .QueryInterface(Ci.calIObvious) everywhere.

At this point, you should probably consider nsIClassInfo deprecated, and likely to go away. Its main uses were meant to deal with exposing things to the web, which we now use WebIDL for. If JS objects are being used by JS, they shouldn't be using XPConnect bindings at all. If they're being used by C++, ClassInfo has no effect.
Flags: needinfo?(kmaglione+bmo)
(In reply to Jorg K (GMT+1) from comment #26)
> https://treeherder.mozilla.org/#/jobs?repo=try-comm-
> central&revision=f87978efb7b891b17c94310ed1305840767861f9
Not successful. I'll do a try for bug 1452120 alone on the beta.
(In reply to [:MakeMyDay] from comment #25)
> Comment on attachment 8973160 [details] [diff] [review]
> Fix - v1
> 
> Review of attachment 8973160 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks for bringing the xpcshell tests back! r+ with the below comments
> considered.
> 
> Xpcshell test are running locally for me with this patch applied on top of
> the patch 1452120, but a try push wouldn't hurt.
> 
> ::: calendar/base/backend/icaljs/calRecurrenceRule.js
> @@ +12,5 @@
> >  }
> >  
> > +var calRecurrenceRuleInterfaces = [
> > +    Components.interfaces.calIRecurrenceRule,
> > +    Components.interfaces.calIRecurrenceItem
> 
> calIReccurrenceItem wasn't included before. Why is that needed now?

calIRecurrenceRule's idl interfaces extends calIRecurrenceItem. This is really just for completeness, because essentially the object supports both interfaces. It is not strictly necessary.


> 
> Apart from that, we should convert Components.interfaces to Ci at all places
> we touch it (you changed this at some places already). But since we have a
> separate bug for it, I spare further comments on that in this review.
Yeah, let's run the script in that separate bug soon.


> 
> ::: calendar/base/modules/calUtils.jsm
> @@ +189,5 @@
> > +        if (aInterfaces.length == 1) {
> > +            cal.WARN("When generating QI for one interface, please use ChromeUtils.generateQI", cal.STACK(10));
> > +            return ChromeUtils.generateQI(aInterfaces);
> > +        } else {
> > +            /* Note that Ci[Ci.x] == Ci.x for all x */
> 
> What's this comment for?
This is copy/paste from XPCOMUtils. I'm not really sure what relevance this has.


> 
> @@ +192,5 @@
> > +        } else {
> > +            /* Note that Ci[Ci.x] == Ci.x for all x */
> > +            let names = [];
> > +            if (aInterfaces) {
> > +                for (let i = 0; i < aInterfaces.length; i++) {
> 
> Can you switch to for (let iface of interfaces) here?
In reading some m-c code, it seems it is advisable to use only js code that is as basic as possible here. This is hot code, and for..of requires the iterator protocol, while for(;;) can be optimized better.

I'd suggest we leave the patch as is given above comments and push when any dependencies are ready.
Still unsuccessful as before:
TypeError: Components.classes["@mozilla.org/calendar/backend-loader;1"] is undefined
ReferenceError: cal is not defined at [snip]/calendar/test/unit/test_duration.js:6
etc.
Hmm wfm locally. I'll try again with the other patches on my stack, maybe one of them has side-effects.
Fails with my try run as well. Works locally with packaged tests. I'm not really sure what else could be the difference to automation. Wild guess the chrome manifest of Lightning is not properly loaded, but I am not sure.
Attached file alarm.txt β€”
(In reply to Philipp Kewisch [:Fallen]  from comment #33)
> Works locally with packaged tests.

mach xpcshell-test comm/calendar/test/unit/test_alarm.js
passes locally for me but throws up heaps of errors, see enclosed:
 0:01.22 pid:4584 Testing initial creation...JavaScript error: c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/_tests/xpcshell/comm/calendar/test/unit/test_alarm.js, line 33: uncaught exception: 3253927937

 0:01.45 pid:4584 JavaScript error: c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/_tests/xpcshell/comm/calendar/test/unit/test_alarm.js, line 339: uncaught exception: 2147500037

 0:07.34 pid:4584 JavaScript error: resource://calendar/modules/ical.js, line 5643: too much recursion

 0:12.61 pid:4584 JavaScript error: c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/_tests/xpcshell/comm/calendar/test/unit/test_alarm.js, line 355: uncaught exception: 2147500037

 0:12.64 pid:4584 JavaScript error: c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/_tests/xpcshell/comm/calendar/test/unit/test_alarm.js, line 409: uncaught exception: 2152071170

just to list a few.
Blocks: 1442985
Try push including the patch from bug 1466430 that corrects an unpacking issue:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=4a98dea55ec958129288d0199422a5c7bc8aa338
If green, we should bring the tests back.
Depends on: 1469499
That's mostly successful, however, during the Calendar outage period, some test failures have crept in:
test_ics_parser.js and test_providers.js.

Geoff, while we had no other workers, I also used to fix Calendar bustage which I didn't enjoy much. So could you look at these, please.

For example, nsIScriptableUnicodeConverter::convertFromByteArray was removed here:
https://hg.mozilla.org/mozilla-central/rev/22c2645c283c
and we didn't notice. I filed bug 1469499 for that.

I haven't looked why test_providers.js fails.
Flags: needinfo?(geoff)
Depends on: 1469501
Another try with the failing tests switched off:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=5a324857a1d33859a186680fb8580ec33e02028d

Hopefully we're good to go then.
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/02830077b19b
Replace use of XPCOMUtils.generateQI where possible. r=MakeMyDay
Philipp, there is no target 6.4 :-(
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: needinfo?(geoff)
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → 6.4
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: