Port |bug 1152332 - Let connection proxy filters return the result asynchronously| to Mailnews - TEST-UNEXPECTED-TIMEOUT | comm/mailnews/compose/test/unit/test_smtpProxy.js

RESOLVED FIXED in Thunderbird 60.0

Status

defect
RESOLVED FIXED
Last year
Last year

People

(Reporter: jorgk, Assigned: mayhemer)

Tracking

Thunderbird 60.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Thunderbird-testfailure: X all][Thunderbird-temporary-fix])

Attachments

(1 attachment, 2 obsolete attachments)

Reporter

Description

Last year
This makes our four proxy tests fail:
TEST-UNEXPECTED-TIMEOUT | comm/mailnews/compose/test/unit/test_smtpProxy.js | Test timed out
TEST-UNEXPECTED-TIMEOUT | comm/mailnews/imap/test/unit/test_imapProxy.js | Test timed out
TEST-UNEXPECTED-TIMEOUT | comm/mailnews/local/test/unit/test_pop3Proxy.js | Test timed out
TEST-UNEXPECTED-TIMEOUT | comm/mailnews/news/test/unit/test_nntpProxy.js | Test timed out
Reporter

Comment 1

Last year
Honza, I'm looking at the tests, for example:
https://dxr.mozilla.org/comm-central/source/mailnews/compose/test/unit/test_smtpProxy.js
and it's rather short.

I'm a little familiar with proxy resolution in mailnews, since I worked on bug 791645. As far as I know, we don't use proxy filters, so it's unclear to my why bug 1152332 affects our tests.

Any hint would be welcome.
Flags: needinfo?(honzab.moz)
Reporter

Updated

Last year
Keywords: leave-open
Whiteboard: [Thunderbird-testfailure: X all] → [Thunderbird-testfailure: X all][Thunderbird-temporary-fix]
Reporter

Comment 2

Last year

Comment 3

Last year
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/9b23f29d39df
temporarily disable proxy tests. rs=bustage-fix
Assignee

Comment 4

Last year
Jorg, I knew I wanted to do one more thing before landing that patch - file a bug for mailnews!  Sorry about that.

I can help fixing this.  Apparently, the code at [1] needs an update, very simple, similarly to [2]:

[1] https://dxr.mozilla.org/comm-central/rev/fdbbb8fee22945c32eeaaa83526b9ad4f360e99a/mailnews/test/resources/NetworkTestUtils.jsm#248
[2] https://hg.mozilla.org/mozilla-central/rev/ce34d962b0df#l5.1
Assignee: nobody → honzab.moz
Flags: needinfo?(honzab.moz)
Reporter

Comment 5

Last year
Oh, I've just written a patch, and I'm checking whether the tests pass now.
Reporter

Comment 6

Last year
Hmm, I ported the two lines from
https://hg.mozilla.org/mozilla-central/rev/ce34d962b0df#l5.12
and the SMTP test, the only one I tried, still hangs. So I must be missing something.

Maybe I should leave it to the assignee ;-) (sorry, I didn't see the Assignee: nobody@mozilla.orghonzab.moz@firemni.cz)
Assignee

Comment 7

Last year
There are two |return| statements in that function ;)  you need to update both.
Assignee

Comment 8

Last year
Posted patch v1Splinter Review
Attachment #8950204 - Attachment is obsolete: true
Assignee

Comment 9

Last year
Comment on attachment 8950211 [details] [diff] [review]
v1

Jorg, from some reason I'm getting "ERROR: Cannot find project mail" when building tbird, so I can't test locally.
Attachment #8950211 - Flags: feedback?(jorgk)
Reporter

Comment 10

Last year
Comment on attachment 8950211 [details] [diff] [review]
v1

All four tests pass with this. Thanks!
I'll get this landed, I hope you're satisfied with my r+. Seems like a straight port of https://hg.mozilla.org/mozilla-central/rev/ce34d962b0df#l5.12 ... if one doesn't overlook half the work like I did ;-(
Attachment #8950211 - Flags: feedback?(jorgk) → review+

Comment 11

Last year
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/782575b2232e
Backed out changeset 9b23f29d39df: re-enable temporarily disabled test. a=jorgk
https://hg.mozilla.org/comm-central/rev/41a8e252b0c2
Port bug 1152332 to mailnews: proxy filters return the result asynchronously. r=jorgk
Reporter

Updated

Last year
Status: NEW → RESOLVED
Closed: Last year
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 60.0
Assignee

Updated

Last year
Attachment #8950012 - Attachment is obsolete: true
Assignee

Comment 12

Last year
Thanks for quickly testing and landing this!  Could not build my tb locally...
Assignee

Updated

Last year
Attachment #8950211 - Attachment description: wip - untested → v1
You need to log in before you can comment on or make changes to this bug.