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

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

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

Comment 1

[Jorg K (CEST = GMT+2)]

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?

Comment 2

[Frank-Rainer Grahl (:frg)]

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?

Comment 3

[Frank-Rainer Grahl (:frg)]

My money would be on Bug 1383215

Comment 4

[Frank-Rainer Grahl (:frg)]

Is 

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

correct? 

Every other import is 

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

Comment 5

[Frank-Rainer Grahl (:frg)]

Same file so maybe the changed cache code gets confused.

Comment 6

[Jorg K (CEST = GMT+2)]

I've backed out bug 1368362 locally and it didn't fix anything.

Comment 7

[Frank-Rainer Grahl (:frg)]

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

Comment 8

[Jorg K (CEST = GMT+2)]

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

Comment 9

[Jorg K (CEST = GMT+2)]

Take a look here, here they add in the "gre":
https://hg.mozilla.org/mozilla-central/rev/df302a197bca

Comment 10

[Frank-Rainer Grahl (:frg)]

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.

Comment 11

[Jorg K (CEST = GMT+2)]

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.

Comment 12

[Frank-Rainer Grahl (:frg)]

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

Comment 13

[Frank-Rainer Grahl (:frg)]

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?

Comment 14

[Jorg K (CEST = GMT+2)]

Attachment: 1384218-import.patch - WIP

This fixes the failing Xpcshell test test_gdata_provider.js

Comment 15

[Jorg K (CEST = GMT+2)]

Attachment: 1384218-import.patch

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.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/0bcc573012c9
Port bug 1383215: Unify module URIs for import. rs=bustage-fix

Comment 17

[Jorg K (CEST = GMT+2)]

(In reply to Frank-Rainer Grahl (:frg) from comment #10)
> Wonder if this doesn't break some other add-ons too.
It broke ThunderHTMLedit :-(

Comment 18

[Jorg K (CEST = GMT+2)]

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.

Comment 19

[Jorg K (CEST = GMT+2)]

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 20

[:aceman]

Comment on attachment 8890093 [details] [diff] [review]
1384218-import.patch

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

This looks correct.

Comment 21

[:aceman]

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

Comment 22

[Jorg K (CEST = GMT+2)]

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

Comment 23

[Jorg K (CEST = GMT+2)]

Attachment: 1384218-import-urls-jsaccount.patch

OK, fixing the imports in JS Account.

Comment 24

[:aceman]

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.

Comment 25

[Jorg K (CEST = GMT+2)]

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

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/cf4f7ca606f3
Follow-up: Fix import URLs in JS Account. r=aceman