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

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.

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.

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

(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.

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 push in comment #11 got us busted completely since calendar is referenced in https://dxr.mozilla.org/comm-central/rev/8d636ad5810b5586433294c16cb2376d826fcca8/mail/locales/Makefile.in#97 Here's an attempt to fix this: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=8b005a969404f8339f346e6c03190b0570a3495e And another one: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=0c6154af9412de561610d6d25ac252d927b7c518

And another one: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=376c61d4e6b1d8c9b0dcab7dc097bede6760d58c

The first failure-messages don't point in that direction, but with the Timeouts, I wonder if this can be related to Bug 1449487?

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.

Started four try runs against various M-C revisions to bisect this: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=43b52aa924f27a76287271dce2abca2af20dff2e https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a1e1237f96f15ffc8279a1f75567b9152d6d2a9b https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=aea78010c8fb1aaadbc3f1a4040ec8415b847ddf https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=00e0976b536107b83df3747aa32761a70aec4c86

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?

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.

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.

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".

(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.

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.

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?

https://hg.mozilla.org/try-comm-central/rev/e51b776a5618ccfdc63eb2e3f770b47e0ddc8fc5 (bug 1452120) https://hg.mozilla.org/try-comm-central/rev/bc5dc60d53bac7dd5ba0da38458376ebf949f3a8 https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f87978efb7b891b17c94310ed1305840767861f9

(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.

(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.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=4e9482e4eac13e8d0a1441530ccae4eb7a02e2b2 with bug 1452120 landed as requested in bug 1452120 comment 42. BTW, replacing Components.interfaces is bug 1458367.

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.

(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.

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.

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.

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 :-(