Closed Bug 1588500 Opened 6 months ago Closed 5 months ago

message-header/test-phishing-bar.js fails and seems to affect other tests in the same directory


(Thunderbird :: General, task)

Not set


(Not tracked)

Thunderbird 72.0


(Reporter: jorgk-bmo, Unassigned)




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


(2 files)

+++ This bug was initially created as a clone of Bug #1570954 +++

As per bug 1570954 comment #13:

Hmm, I tried to run the entire directory message-header and crashed on test-phishing-bar.js with:

TEST-START | c:\mozilla-source\comm-central\comm\mail\test\mozmill\message-header\test-phishing-bar.js | test_no_phishing_warning_for_ip_sameish_text
[5788, Main Thread] WARNING: '!ClientIsValidPrincipalInfo(mClientInfo.PrincipalInfo())', file c:/mozilla-source/comm-central/dom/clients/manager/ClientSource.cpp, line 176
++DOMWINDOW == 84 (000001FC4CD1C000) [pid = 5788] [serial = 95] [outer = 000001FC493B05C0]
[5788, Main Thread] WARNING: Failed to retarget HTML data delivery to the parser thread.: file c:/mozilla-source/comm-central/parser/html/nsHtml5StreamParser.cpp, line 1142
Assertion failure: (((HRESULT)(hr)) >= 0), at c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/dist/include\mozilla/ShellHeaderOnlyUtils.h:132
#01: nsExternalHelperAppService::LoadURI (c:\mozilla-source\comm-central\uriloader\exthandler\nsExternalHelperAppService.cpp:966)
#02: XPTC__InvokebyIndex[c:\mozilla-source\comm-central\obj-x86_64-pc-mingw32\dist\bin\xul.dll +0x8dd6c62]
#03: CallMethodHelper::Call (c:\mozilla-source\comm-central\js\xpconnect\src\XPCWrappedNative.cpp:1183)
#04: XPCWrappedNative::CallMethod (c:\mozilla-source\comm-central\js\xpconnect\src\XPCWrappedNative.cpp:1149)
#05: XPC_WN_CallMethod (c:\mozilla-source\comm-central\js\xpconnect\src\XPCWrappedNativeJSOps.cpp:946)

I took some subtests of test-phishing-bar.js out and now the entire directory passes.

Here's a try run with that:

Looks like more fallout from bug 1578624, like we already have bug 1588256.

Also seen in debug runs as:
Assertion failure: (((HRESULT)(hr)) >= 0), at z:/build/build/src/obj-thunderbird/dist/include\mozilla/ShellHeaderOnlyUtils.h:132

Pushed by
disable failing subtests of test-phishing-bar.js. rs=bustage-fix

Closed: 6 months ago
Resolution: --- → FIXED
Ever confirmed: false
Resolution: FIXED → ---
Ever confirmed: true

Unless I'm mistaken, the
Assertion failure: (((HRESULT)(hr)) >= 0), at z:/build/build/src/obj-thunderbird/dist/include\mozilla/ShellHeaderOnlyUtils.h:132
is first seen here:
on Wed, Aug 28, 2019, 19:29:02.

And it dies in test-phishing-bar.js:

Running the test manually, those subtest open tabs in the default browser Firefox, and I don't think that's intended.

So this is a quite long-standing issue which has been ignored :-(

It looks like it's from bug 1570845. But I think our test is wrong. Opening the external browser should be dummied up.

Aceman, what does the test do on Linux?

Looking at this again, I guess it was wrong to disable the tests completely, they should have just been disabled on Windows.

Flags: needinfo?(acelists)
Regressed by: 1570845

Pushed by
Disable subtests of test-phishing-bar.js on Windows only. rs=bustage-fix DONTBUILD

Closed: 6 months ago6 months ago
Resolution: --- → FIXED

Aceman ran the test for me and reported that his default browser got opened. I don't think this is a good thing to do in automation since there might not even be a default browser configured. In automation the test only crashes on Windows 10 debug.

Toshi, why do we assert here
if it's not fatal? Under which circumstances can that happen?

The test simulates a click on a link which launches an external browser.

Flags: needinfo?(acelists) → needinfo?(tkikuchi)

Toshi, why do we assert here
if it's not fatal? Under which circumstances can that happen?

We're not sure when CoAllowSetForegroundWindow fails, and that's why we added MOZ_ASSERT. Let me run message-header/test-phishing-bar.js and see there is anything we should do in ShellExecuteByExplorer.

I'm in the training this week. I'll keep needinfo? until I start looking into this.

Thanks, if you need any assistance with running the test (or anything else related to TB), please let me know.

This directory's been failing like mad on Linux, and it started with the push in comment 4.

Resolution: FIXED → ---
Pushed by
Disable subtests of test-phishing-bar.js for Linux as well as Windows. rs=bustage-fix DONTBUILD

Looks like the push in comment #4 closed this inadvertently.


So if this fails on Linux, can a developer in Linux fix it? According to comment #5 it was launching Aceman's default browser. My suggestion would be to dummy-up launching the browser in the first place since we don't know how automation servers are set up.

Flags: needinfo?(geoff)
Regressions: 1591357
No longer regressions: 1591357

Preventing the browser from (trying) to open fixes the problems at least locally. Let's see about try:

Well, if you're not opening in an external browser, you might as well enable Windows, too.

Flags: needinfo?(geoff)
Assignee: nobody → mkmelin+mozilla
Attachment #9104313 - Flags: review?(jorgk)

In cases like this one, an artifact build is much faster:
try: -b o -p linux64-shippable,macosx64-shippable,win64-shippable -u mozmill-1proc --artifact
Or you can also do -u all.

I could have checked the test before landing (as I will anyway).

Comment on attachment 9104313 [details] [diff] [review]

Thanks. That will make Toshihito's life a little harder, but he can just back that out locally.
Attachment #9104313 - Flags: review?(jorgk) → review+

Pushed by
fix message-header/test-phishing-bar.js by avoiding opening an external browser. r=jorgk

Closed: 6 months ago6 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 72.0
Attachment #9104313 - Flags: approval-comm-beta+

I was a bit foolish landing this. Both try runs show Z4 failures:

Yes, message-header/test-phishing-bar.js runs through, but if you run the entire directory like so
mozmake SOLO_TEST=message-header mozmill-one
you get this error:

TEST-UNEXPECTED-FAIL | Z:\task_1572037219\build\tests\mozmill\message-header\test-reply-identity.js | test-reply-identity.js::test_reply_no_matching_identity

That can be reproduced locally.

Flags: needinfo?(mkmelin+mozilla)
Resolution: FIXED → ---

Backed out changeset b964b4e0f62f (bug 1588500) for MozMill test failures. a=backout

Sorry, Magnus, your changes for the phishing test seem fine and the test itself passes and you suppress opening the external browser. But there is a gremlin in the directory, see comment #19. Geoff also noticed that, see comment #8 :-(

Attachment #9104313 - Flags: approval-comm-beta+

Locally (with and without the patch) the whole directory works for me on linux.

I do see likely unrelated console spew about (xref bug 1446699)
GLib-GObject-WARNING **: 14:39:19.132: ../../../gobject/gsignal.c:3498: signal name 'selection_changed' is invalid for instance '0x7f4eba480110' of type 'MaiAtkType3'

Assignee: mkmelin+mozilla → nobody
Flags: needinfo?(mkmelin+mozilla)

Hi Toshihito, are you sure your try run is want you really wanted to do? The second backout restores the
disabled_test_ ... functions, so the test will still not run. To go back to the beginning, you also need to backout the changeset from comment #2.

Sorry, your try got caught up in total bustage which has been fixed now.

What do you want to achieve with a try? IIRC, the issue is intermittent but fails each time on a local build. If you just want to inspect a log, turn to comment #2.

I want to repro the assert of ShellHeaderOnlyUtils.h. I ran mozmake SOLO_TEST=message-header mozmill-one locally with the latest build (without any backout) , but I didn't get a repro. What should I do to get a repro on top of the latest build?

Flags: needinfo?(jorgk)

Hi Toshihito, thanks for your persistence. With no backout, the test is disabled. You'd have to apply the three backouts from your latest try, or simple use the folded patch I'm providing here for your convenience. It simply removes all the disabling that we tweaked in those three changesets (first on all platforms, then only on Windows, last on Windows and Linux since Linux also seems to have a problem).

In short, with the patch applied running
mozmake SOLO_TEST=message-header/test-phishing-bar.js mozmill-one
I immediately get
Assertion failure: (((HRESULT)(hr)) >= 0), at c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/dist/include\mozilla/ShellHeaderOnlyUtils.h:132
on Windows 10 x64 Pro (1903).

I believe that in automation I've seen the issue come and go, but locally it fails 100%.

Off-topic: Looks like bug 1567614 has raised its ugly head again :-(

Assignee: nobody → jorgk
Flags: needinfo?(jorgk)
Assignee: jorgk → nobody

Thank you for providing the detail information! I've got the repro and I think I got the root cause as well. I filed a new bug 1592444 to track this assert issue.

Flags: needinfo?(tkikuchi)

Sigh, running this with Magnus patch, attachment 9104313 [details] [diff] [review], still gives:

SUMMARY-UNEXPECTED-FAIL | c:\mozilla-source\comm-central\comm\mail\test\mozmill\message-header\test-reply-identity.js | test-reply-identity.js::test_reply_no_matching_identity
  EXCEPTION: controller(): Window could not be initialized.
    at: utils.jsm line 390
       TimeoutError utils.jsm:390 13
       waitFor utils.jsm:446 11
       MozMillController controller.jsm:353 9
       WindowWatcher_waitForWindowOpen WindowHelpers.jsm:291 13
       wait_for_new_window WindowHelpers.jsm:642 19
       wait_for_compose_window ComposeHelpers.jsm:265 34
       open_compose_with_reply ComposeHelpers.jsm:84 10
       test_reply_no_matching_identity test-reply-identity.js:156 18

Running it on Windows with all the tests enabled, runs through.

Let's see:

Pushed by
Re-enable test-phishing-bar.js on Linux and Windows. r=me

Closed: 6 months ago5 months ago
Resolution: --- → FIXED

We'll see whether it raised its ugly head again on Linux. Windows should be fine after bug 1592444.

You need to log in before you can comment on or make changes to this bug.