Closed Bug 1384218 Opened 3 years ago Closed 3 years ago

Massive Mozmill and Xpcshell failure on 2017-07-25 (40 failing tests)

Categories

(Thunderbird :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 56.0

People

(Reporter: jorgk-bmo, Assigned: jorgk-bmo)

References

Details

Attachments

(2 files, 1 obsolete file)

Failing Xpcshell:

TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_gdata_provider.js | xpcshell return code: 0
ReferenceError: MockRegistrar is not defined at C:/slave/test/build/tests/xpcshell/tests/calendar/test/unit/test_gdata_provider.js:66

Failing Mozmill:
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-mail-account-setup-wizard.js | test-mail-account-setup-wizard.js::test_mail_account_setup
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-mail-account-setup-wizard.js | test-mail-account-setup-wizard.js::test_bad_password_uses_old_settings
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-retest-config.js | test-retest-config.js::test_re_test_config
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\folder-display\test-message-commands-on-msgstore.js | test-message-commands-on-msgstore.js::<TOP_LEVEL>
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\folder-display\test-message-commands-on-msgstore.js | test-message-commands-on-msgstore.js::test_mark_messages_read
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\folder-display\test-message-commands-on-msgstore.js | test-message-commands-on-msgstore.js::test_mark_messages_flagged
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\folder-display\test-message-commands-on-msgstore.js | test-message-commands-on-msgstore.js::test_mark_messages_replied
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\folder-display\test-message-commands-on-msgstore.js | test-message-commands-on-msgstore.js::test_mark_messages_forwarded
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\instrumentation\test-instrument-setup.js | test-instrument-setup.js::test_mail_account_setup
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\instrumentation\test-instrument-setup.js | test-instrument-setup.js::teardownModule
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\message-header\test-header-toolbar.js | test-header-toolbar.js::test_customize_header_toolbar_remove_buttons
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_get_an_account
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_restored_ap_tab_works
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_can_dismiss_account_provisioner
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_can_switch_to_existing_email_account_wizard
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_can_display_providers_in_other_languages
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_flip_flop_from_provisioner_menuitem
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_persist_name_in_search_field
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_html_characters_and_ampersands
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_show_tos_privacy_links_for_selected_providers
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_shows_error_on_bad_suggest_from_name
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_shows_error_on_empty_suggest_from_name
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_throws_console_error_on_corrupt_XML
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_broken_provider_list_goes_offline
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_incomplete_provider_not_displayed
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_search_button_disabled_cases
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_other_lang_link_hides
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_internal_link_opening_behaviour
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_window_open_link_opening_behaviour
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_external_link_opening_behaviour
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_return_to_provisioner_on_error_XML
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_disabled_fields_when_searching
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_search_button_disabled_if_no_lang_support
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_provider_language_wildcard
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_search_button_disabled_if_no_query_on_init
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_get_new_account_focuses_existing_ap_tab
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\newmailaccount\test-newmailaccount.js | test-newmailaccount.js::test_per_address_prices
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\session-store\test-session-store.js | test-session-store.js::<TOP_LEVEL>
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\session-store\test-session-store.js | test-session-store.js::setupModule

M-C last good: 131e19a573e901fb4d01b471b11b791642
M-C first bad: 07484bfdb96bc7297c404e377eea93f1d8

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=131e19a573e901fb4d01b471b11b791642&tochange=07484bfdb96bc7297c404e377eea93f1d8
To start somewhere I've run
mozmake SOLO_TEST=session-store/test-session-store.js mozmill-one
locally and get:
  EXCEPTION: sessionStoreManager is not defined
    at: test-session-store.js line 88
       setupModule test-session-store.js:88 3

sessionStoreManager is defined in sessionStoreManager.js which is imported.

mozmake SOLO_TEST=folder-display/test-message-commands-on-msgstore.js mozmill-one
gives:
  EXCEPTION: statusHeader is undefined
    at: test-message-commands-on-msgstore.js line 92
       check_status test-message-commands-on-msgstore.js:92 3
       test_mark_messages_read test-message-commands-on-msgstore.js:107 3

  EXCEPTION: nsMsgMessageFlags is undefined
    at: test-message-commands-on-msgstore.js line 150
       test_mark_messages_flagged test-message-commands-on-msgstore.js:150 3

Strange because test-message-commands-on-msgstore.js has at the top:
var statusHeader = "X-Mozilla-Status: ";
var nsMsgMessageFlags = Ci.nsMsgMessageFlags;

Really looks like JS is playing a trick on us here.

Arai, can you see something? It wouldn't be from
9315eabc7fb0 Jan de Mooij - Bug 1368362 - Use setSlotWithType instead of setSlot in InitGlobalLexicalOperation. r=shu

This makes me really nervous that TB is really broken. For example I see in the Error console:
TypeError: calendarWindowPrefs is undefined calendar-chrome-startup.js when it is defined.

Is is possible that you broke JS just a little bit?
Flags: needinfo?(jdemooij)
Flags: needinfo?(arai.unmht)
I am seeing:

Timestamp: 7/25/2017, 8:57:19 PM
Error: NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIXPCComponents_Utils.import]
Source File: chrome://calendar/content/calendar-chrome-startup.js
Line: 5

Timestamp: 7/25/2017, 8:57:19 PM
Error: NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIXPCComponents_Utils.import]
Source File: chrome://calendar/content/calendar-dialog-utils.js
Line: 12
. 

Everything else seems to be ok in SeaMonkey so maybe add-on related?
My money would be on Bug 1383215
Is 

> Components.utils.import("resource://gre/modules/iteratorUtils.jsm");

correct? 

Every other import is 

> Components.utils.import("resource://modules/iteratorUtils.jsm");
Same file so maybe the changed cache code gets confused.
I've backed out bug 1368362 locally and it didn't fix anything.
Now 90% sure it is Bug 1383215

> This also ensures that multiple URLs are not used to load the same module, 
> which would result in it being loaded more than once in the new regime

https://hg.mozilla.org/integration/mozilla-inbound/rev/03777f604c6ca9dd56de4d8314284f7303fda46b
Flags: needinfo?(jdemooij)
-Components.utils.import("resource://gre/modules/iteratorUtils.jsm");
+Components.utils.import("resource:///modules/iteratorUtils.jsm");
in the two calendar files doing it this way and the two tests mentioned in comment #1 fail just the same.

And
SUMMARY-UNEXPECTED-FAIL | c:\mozilla-source\comm-central\mail\test\mozmill\folder-display\test-message-commands-on-msgstore.js | test-message-commands-on-msgstore.js::<TOP_LEVEL>
  EXCEPTION: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIXPCComponents_Utils.import]
    at: nonesuch line 19
is also still there.
It probably doesn't matter as long as all imports use the same URL:

This one likely fails now too:

> calendar/providers/gdata/modules/OAuth2.jsm:
>    Cu.import("resource:///modules/XPCOMUtils.jsm");

Looks like we need to fix up the whole tree first. Wonder if this doesn't break some other add-ons too.
I noticed that the failing test_gdata_provider.js uses Components.utils.import("resource:///modules/Services.jsm"); where the rest of the tree uses "gre".

Looks like I'll have to add the "gre" everywhere where we have ///modules/ only.
Flags: needinfo?(arai.unmht)
> +Components.utils.import("resource:///modules/iteratorUtils.jsm");

worked for me.

Now at:

Timestamp: 7/25/2017, 10:08:53 PM
Error: NS_ERROR_FILE_NOT_FOUND: Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]
Source File: chrome://calendar/content/calendar-dialog-utils.js
Line: 10

and then so on...
Looking at:

https://dxr.mozilla.org/comm-central/search?q=%2F%2Fmodules&redirect=false

I wonder why some of the imports do not fail in Gecko? Not always consistent. Is because of only 2 slashes or gre or both?
Attached patch 1384218-import.patch - WIP (obsolete) — Splinter Review
This fixes the failing Xpcshell test test_gdata_provider.js
This fixes
test_gdata_provider.js
test-session-store.js
test-mail-account-setup-wizard.js
test-newmailaccount.js
test-retest-config.js
test-header-toolbar.js
test-instrument-setup.js
test-message-commands-on-msgstore.js

So unless I missed something, I'm done here.
Attachment #8890077 - Attachment is obsolete: true
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/0bcc573012c9
Port bug 1383215: Unify module URIs for import. rs=bustage-fix
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Assignee: nobody → jorgk
Target Milestone: --- → Thunderbird 56.0
(In reply to Frank-Rainer Grahl (:frg) from comment #10)
> Wonder if this doesn't break some other add-ons too.
It broke ThunderHTMLedit :-(
Components.utils.import('resource://gre/modules/iteratorUtils.jsm');
Components.utils.import('resource://gre/modules/mailServices.js');

In TB we now use these without the "gre" everywhere. That makes me think that we might want to insert "gre" everywhere.
While we're in clean-up mode here, all imports of JSAccountUtils.jsm and JaBaseUrl.jsm should lose their "gre" since they are not from toolkit, see bug 1383215 comment #24.
Comment on attachment 8890093 [details] [diff] [review]
1384218-import.patch

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

This looks correct.
Attachment #8890093 - Flags: review+
(In reply to Jorg K (GMT+2) from comment #18)
> Components.utils.import('resource://gre/modules/iteratorUtils.jsm');
> Components.utils.import('resource://gre/modules/mailServices.js');
> 
> In TB we now use these without the "gre" everywhere.

These two shouldn't have 'gre'.

> That makes me think
> that we might want to insert "gre" everywhere.

Why?

We use '///modules' for modules from c-c and '//gre/modules' for m-c modules.
(In reply to :aceman from comment #21)
> We use '///modules' for modules from c-c and '//gre/modules' for m-c modules.
Yes, I learned that at 1 AM last night, see bug 1383215 comment #24.
OK, fixing the imports in JS Account.
Attachment #8890219 - Flags: review?(acelists)
Comment on attachment 8890219 [details] [diff] [review]
1384218-import-urls-jsaccount.patch

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

The tests pass for me with and without the patch. But it seems semantically more correct.
Attachment #8890219 - Flags: review?(acelists) → review+
(In reply to :aceman from comment #24)
> The tests pass for me with and without the patch.
Yes.

> But it seems semantically more correct.
Quoting bug 1383215 comment #24:
But the code that's using resource://gre/ URLs for app content, or vice versa, is still technically wrong.

So let's get it right before M-C implements other checks and we get bustage again. This whole bug happened since people didn't pay attention to the details and it only breaks at midnight when correctness suddenly gets enforced.
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/cf4f7ca606f3
Follow-up: Fix import URLs in JS Account. r=aceman
Depends on: 1388060
You need to log in before you can comment on or make changes to this bug.