1455536 - Massive Xpcshell test failure on 2018-04-19: 47 failing tests (37 mailnews, 5 calendar, 1 chat) - caused by task not allowed to be generators

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapFilterActions.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapFilterActions.js | MoveToFolder - [MoveToFolder : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js | DoNothing - [DoNothing : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapFilterActions.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapFilterActions.js | MoveToFolder - [MoveToFolder : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:comm/calendar/test/unit/test_alarmutils.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:comm/calendar/test/unit/test_alarmutils.js | test_setDefaultValues_events - [test_setDefaultValues_events : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:comm/calendar/test/unit/test_datetimeformatter.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:comm/calendar/test/unit/test_datetimeformatter.js | formatDate_test - [formatDate_test : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:comm/calendar/test/unit/test_deleted_items.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:comm/calendar/test/unit/test_deleted_items.js | test_deleted_items - [test_deleted_items : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:comm/calendar/test/unit/test_ltninvitationutils.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:comm/calendar/test/unit/test_ltninvitationutils.js | getItipHeader_test - [getItipHeader_test : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:comm/calendar/test/unit/test_timezone_definition.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:comm/calendar/test/unit/test_timezone_definition.js | version_test - [version_test : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | xpcshell-libical.ini:comm/calendar/test/unit/test_alarmutils.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | xpcshell-libical.ini:comm/calendar/test/unit/test_alarmutils.js | test_setDefaultValues_events - [test_setDefaultValues_events : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | xpcshell-libical.ini:comm/calendar/test/unit/test_datetimeformatter.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | xpcshell-libical.ini:comm/calendar/test/unit/test_datetimeformatter.js | formatDate_test - [formatDate_test : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | xpcshell-libical.ini:comm/calendar/test/unit/test_deleted_items.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | xpcshell-libical.ini:comm/calendar/test/unit/test_deleted_items.js | test_deleted_items - [test_deleted_items : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | xpcshell-libical.ini:comm/calendar/test/unit/test_ltninvitationutils.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | xpcshell-libical.ini:comm/calendar/test/unit/test_ltninvitationutils.js | getItipHeader_test - [getItipHeader_test : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | xpcshell-libical.ini:comm/calendar/test/unit/test_timezone_definition.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | xpcshell-libical.ini:comm/calendar/test/unit/test_timezone_definition.js | version_test - [version_test : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/chat/components/src/test/test_logger.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/chat/components/src/test/test_logger.js | test_getLogFolderPathForAccount - [test_getLogFolderPathForAccount : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/base/test/unit/test_imapPump.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/base/test/unit/test_imapPump.js | loadImapMessage - [loadImapMessage : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/base/test/unit/test_searchChaining.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/base/test/unit/test_searchChaining.js | setupFolder - [setupFolder : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_attachment.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_attachment.js | testInput0 - [testInput0 : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_bug155172.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_bug155172.js | - Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_detectAttachmentCharset.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_detectAttachmentCharset.js | testUTF8 - [testUTF8 : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_messageHeaders.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_messageHeaders.js | testEnvelope - [testEnvelope : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_saveDraft.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_saveDraft.js | checkDraft - [checkDraft : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_smtpPassword2.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_smtpPassword2.js | - Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_sendObserver.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_sendObserver.js | testObserver - [testObserver : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_smtpPassword.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_smtpPassword.js | - Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_smtpPasswordFailure1.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_smtpPasswordFailure1.js | - Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_smtpPasswordFailure3.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_smtpPasswordFailure3.js | - Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_smtpPasswordFailure2.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_smtpPasswordFailure2.js | - Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_smtpProxy.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_smtpProxy.js | setup - [setup : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_longLines.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/compose/test/unit/test_longLines.js | testBodyWithLongLine - [testBodyWithLongLine : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_filterNeedsBody.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_filterNeedsBody.js | runFilterAction - [runFilterAction : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapMove.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapMove.js | startTest - [startTest : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapPasswordFailure.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapPasswordFailure.js | - Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapProxy.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapProxy.js | setup - [setup : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_filterNeedsBody.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_filterNeedsBody.js | runFilterAction - [runFilterAction : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js | DoNothing - [DoNothing : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapMove.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/imap/test/unit/test_imapMove.js | startTest - [startTest : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_duplicateKey.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_duplicateKey.js | runPump - [runPump : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_movemailDownload.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_movemailDownload.js | streamMessages - [streamMessages : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_pop3MultiCopy2.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_pop3MultiCopy2.js | setupFolders - [setupFolders : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_pop3MultiCopy.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_pop3MultiCopy.js | runPump - [runPump : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_pop3Password2.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_pop3Password2.js | - Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_pop3Password.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_pop3Password.js | - Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_pop3Password3.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_pop3Password3.js | - Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_pop3PasswordFailure.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_pop3PasswordFailure.js | - Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_pop3Proxy.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_pop3Proxy.js | setup - [setup : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_pop3Pump.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/local/test/unit/test_pop3Pump.js | runPump - [runPump : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/mime/test/unit/test_structured_headers.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/mime/test/unit/test_structured_headers.js | check_addressing - [check_addressing : 1] Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/news/test/unit/test_nntpPassword2.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/news/test/unit/test_nntpPassword2.js | - Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/news/test/unit/test_nntpGroupPassword.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/news/test/unit/test_nntpGroupPassword.js | - Task returned a generator - false == true
TEST-UNEXPECTED-FAIL | comm/mailnews/news/test/unit/test_nntpPassword.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/news/test/unit/test_nntpPassword.js | - Task returned a generator - false == true

Due to bustage in bug 1455221, we have a very big regression interval:
M-C last good: 789e30ff2e3d6e1fcfce1a373c1e563548
M-C first bad: be79666e4a595570642c9e8a0097e975c2 (used on try)
That was build on an M-C try push based on 8ed49dd81059

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=789e30ff2e3d6e1fcfce1a373c1e563548&tochange=8ed49dd81059

That looks like bug 1453881.

So we need to do change "function *" to "async function" and replace "yield" with "await".

Comment 1

[Jorg K (CEST = GMT+2)]

Attachment: 1455536-remove-generators.patch - WIP

I've changed two test to see and now both fail with:

 0:01.06 pid:11740 Couldn't convert chrome URL: chrome://messenger/locale/messenger.properties
 0:01.06 pid:11740 [11740, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x8000FFFF: file c:/mozilla-source/comm-central/mailnews/base/src/nsMsgAccountManager.cpp, line 2313
 0:01.08 INFO (xpcshell/head.js) | test run_next_test 0 finished (2)
 0:01.08 ERROR Unexpected exception NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIMsgAccountManager.createLocalMailAccount]
loadLocalMailAccount@resource://testing-common/mailnews/localAccountUtils.js:48:5

nsMsgAccountManager.cpp, line 2313 does:
  rv = GetLocalFoldersPrettyName(localFoldersName);
  NS_ENSURE_SUCCESS(rv, rv);

That function is accessing some string from messenger.properties:
 rv = sBundleService->CreateBundle("chrome://messenger/locale/messenger.properties", ...)

Kris and Florian, are there further restrictions now or is this an unrelated problem?

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: 1455536-remove-generators-in-calendar.patch

I guess in calendar might not run into the same problems, so let's see what that gives.

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: 1455536-remove-generators-in-chat.patch

I'll try it when my local build is finished.

Comment 4

[:aceman]

Just great. I thought generators were the new trend? Are they just not allowed in some "task" context? What is it?

Comment 5

[Jorg K (CEST = GMT+2)]

Those two patches fix the chat test and the calendar tests, however, calendar/test/unit/test_ltninvitationutils.js now fails with

 0:17.35 ERROR Unexpected exception TypeError: Components.classes['@mozilla.org/xmlextras/xmlserializer;1'] is undefined at resource://calendar/modules/utils/calXMLUtils.jsm:172

 0:17.35 INFO "CONSOLE_MESSAGE: (warn) [JavaScript Warning: "ReferenceError: reference to undefined property "@mozilla.org/xmlextras/xmlserializer;1"" {file: "resource://calendar/modules/utils/calXMLUtils.jsm" line: 172}]"

Comment 6

[Jorg K (CEST = GMT+2)]

Forget comment #5. This already got replaced in bug 1452352, but due to recent massive Calendar refactoring the old call got reinstated :-(

You have to look twice to see it:
Fixed file: calendar/base/modules/calXMLUtils.jsm
https://hg.mozilla.org/comm-central/rev/78c17f98345c#l1.33

Popped up gain in calendar/modules/utils/calXMLUtils.jsm :-(

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/bfc6cf2ec5b3
Port bug 1453881: add_task() doesn't accept generators any more (calendar part). rs=bustage-fix
https://hg.mozilla.org/comm-central/rev/e3d205e2a797
Port bug 1453881: add_task() doesn't accept generators any more (chat part). rs=bustage-fix CLOSED TREE

Comment 8

[Jorg K (CEST = GMT+2)]

I miscounted that since lines were repeated in the log. With these pushes, we are down to 41 mailnews failures. I'll do a patch to see whether doing the textual replacements will fix anything.

Comment 9

[Jorg K (CEST = GMT+2)]

Attachment: 1455536-remove-generators.patch

That fixes 37 mailnews tests: Let's see:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=57a146f6f2b702c9b63b988c1bf9104d8838efeb

Comment 10

[Jorg K (CEST = GMT+2)]

Attachment: 1455536-remove-generators.patch - more, but maybe nor necessary

This is really painful since it's not obvious where to do the textual replacement. I thought I had missed test_pop3PasswordFailure2.js and test_pop3PasswordFailure3.js but that doesn't fail, at least not according to comment #0.

Even after this replacement, there are still heaps of generators left and the matching yields. I guess, those don't use tasks.

Comment 11

[Florian Quèze [:florian]]

Comment on attachment 8969599 [details] [diff] [review]
1455536-remove-generators-in-chat.patch

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

::: chat/components/src/test/test_logger.js
@@ +408,5 @@
>    }
>  
>    // Remove the created log files, testing forEach in the process.
> +  await logger.forEach({
> +    processLog: Task.async(async function (aLog) {

Please remove "Task.async(" here, and the matching ")".
So this line becomes:
  async processLog(aLog) {

Task.async converts a generator function into a task. An async function can directly be used as a task.

@@ +434,5 @@
>      originalMessage: "Hello, world!",
>      outgoing: true
>    };
>  
> +  let logMessage = Task.async(async function (aMessage) {

Task.async here too.

@@ +448,5 @@
>    notEqual(logWriter.currentPath, oldPath);
>    // The log writer's new start time should be the time of the message.
>    equal(message.time * 1000, logWriter._startTime);
>  
> +  let getCurrentHeader = Task.async(async function () {

and here.

Comment 12

[Florian Quèze [:florian]]

(In reply to Jorg K (GMT+1) from comment #1)

> Kris and Florian, are there further restrictions now or is this an unrelated
> problem?

Probably an unrelated problem.

Comment 13

[Jorg K (CEST = GMT+2)]

I don't see that "unrelated failure any more" in the latest try run.

Comment 14

[Kris Maglione [:kmag]]

(In reply to :aceman from comment #4)
> Just great. I thought generators were the new trend? Are they just not
> allowed in some "task" context? What is it?

Generators continue to be supported, of course. They just can't be used in test tasks as if they were async functions anymore. The polyfill that supported that has been a performance problem and maintenance burden, so it's going away.

Comment 15

[Jorg K (CEST = GMT+2)]

Attachment: 1455536-remove-generators.patch (v3)

Missed two and reverted to incorrect ones:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=52ffb7da8cf75693296b59f76e756417b131c053

Comment 16

[Jorg K (CEST = GMT+2)]

Attachment: 1455536-remove-generators.patch (v4)

Actually, maybe the previous on was wrong, here's another one:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=6f813256a9590e927dda2de07c41743739a976c4

Comment 17

[Jorg K (CEST = GMT+2)]

Attachment: 1455536-remove-generators.patch (v5)

Removing Task.async() here as well as per Florian's review for chat.
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=94504f3356380651407d04df944f489bec7fc357

Comment 18

[Jorg K (CEST = GMT+2)]

Attachment: 1455536-remove-task-async.patch

You mentioned three of the five Task.async() calls. I removed all five now, the test still passes.

Please convert the r- on the other patch to an f+ since this already landed. It was the correct ad hoc fix and I'm following up here.

Comment 19

[Florian Quèze [:florian]]

Comment on attachment 8969732 [details] [diff] [review]
1455536-remove-task-async.patch

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

::: chat/components/src/test/test_logger.js
@@ +408,5 @@
>    }
>  
>    // Remove the created log files, testing forEach in the process.
>    await logger.forEach({
> +    processLog: async function (aLog) {

The usual format these days is:
  async processLog(aLog) {

Comment 20

[Florian Quèze [:florian]]

mailnews has a few more Task.jsm usages that would be good to remove too to avoid having bustage there soon: https://searchfox.org/comm-central/search?q=%5CbTask%5C.(async%7Cspawn)&case=false&regexp=true&path=mailnews

Comment 21

[Jorg K (CEST = GMT+2)]

I removed the one in test_movemailDownload.js. The other ones are Task.spawn() and I'm not even sure that the generator will need replacing there. One try run here replaces the generator in test_imapFilterActions.js and test_imapFilterActionsPostplugin.js but I reverted that in the next try. I'm just trying to fix the bustage here without much insight sadly. I'm sure I'm missing something.

Comment 22

[Jorg K (CEST = GMT+2)]

Attachment: 1455536-remove-task-async.patch

Switched to async processLog(aLog).

Comment 23

[Jorg K (CEST = GMT+2)]

v3, v4 and v5 all worked. So let's go with v3 which removes more generators and add the removal of Task.async() added in v5.

Comment 24

[Jorg K (CEST = GMT+2)]

Attachment: 1455536-remove-generators.patch (v3b)

OK, going back to v3.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/bb98b2c207be
Port bug 1453881: add_task() doesn't accept generators any more (chat part, take 2). r=florian
https://hg.mozilla.org/comm-central/rev/445b9471aa3a
Port bug 1453881: add_task() doesn't accept generators any more (mailnews part). rs=bustage-fix CLOSED TREE

Comment 26

[Jorg K (CEST = GMT+2)]

We can follow up the remaining Task.async() and Task.spawn() in another bug or here:
https://searchfox.org/comm-central/search?q=%5CbTask%5C.(async%7Cspawn)&case=false&regexp=true&path=mailnews

Aceman, care to take a look?

Comment 27

[Jorg K (CEST = GMT+2)]

Comment on attachment 8969599 [details] [diff] [review]
1455536-remove-generators-in-chat.patch

Turning this into an r+ since it was landed before the review and the issues were fixed in a follow-up. Otherwise it will confuse posterity.

Comment 28

[Jorg K (CEST = GMT+2)]

Florian, can you please give us some more tips here.
https://searchfox.org/comm-central/search?q=%5CbTask%5C.(async%7Cspawn)&case=false&regexp=true&path=mailnews
now shows four call sites.
  run: Task.async(function *() {
becomes
  async run() {
but what about the |Task.spawn(async function () {|?

Comment 29

[Tooru Fujisawa [:arai]]

just `(async function() { ... })()`

Comment 30

[Jorg K (CEST = GMT+2)]

OK, I'll do a patch.

Comment 31

[Jorg K (CEST = GMT+2)]

Attachment: 1455536-async-spawn.patch

One review will do, whoever comes first ;-)

I ran
mach xpcshell-test comm/mailnews/mime/jsmime/test
mach xpcshell-test comm/mailnews/imap/test/unit/test_imapFilterActions.js
mach xpcshell-test comm/mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js
and they still pass.

Comment 32

[Tooru Fujisawa [:arai]]

Comment on attachment 8970084 [details] [diff] [review]
1455536-async-spawn.patch

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

::: mailnews/base/src/msgAsyncPrompter.js
@@ +15,5 @@
>  runnablePrompter.prototype = {
>    _asyncPrompter: null,
>    _hashKey: null,
>  
> +  async run() {

you should remove the ")" from the end of the function, which corresponds to "(" of "Task.async(".

::: mailnews/imap/test/unit/test_imapFilterActions.js
@@ +387,5 @@
>  
>  // basic preparation done for each test
>  function setupTest(aFilter, aAction)
>  {
> +  return (async function () {

actually, you can just make the enclosing function async, like:
  async function setupTest(aFilter, aAction) {
    ...
  }
with body for the inner async function.
that has the same effect.

but that reduces the indentation for inner function body, so this would also be fine.

::: mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js
@@ +216,5 @@
>  
>  // basic preparation done for each test
>  function setupTest(aFilter, aAction)
>  {
> +  return (async function () {

same here.

::: mailnews/mime/jsmime/test/head_xpcshell_glue.js
@@ +114,5 @@
>  };
>  
>  /// The outer call to run a test suite, which returns a promise of completion.
>  MochaSuite.prototype.runSuite = function () {
> +  return this._runSuite.bind(this);

this doesn't call the _runSite function.
_runSuite needs to be rewritten as async function, and you should call like:
  return this._runSuite();

Comment 33

[:aceman]

Comment on attachment 8969750 [details] [diff] [review]
1455536-remove-generators.patch (v3b)

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

Thanks.

::: mailnews/compose/test/unit/test_bug155172.js
@@ +32,5 @@
>  // is intentionally wrong.
>  var kPassword1 = "wrong";
>  var kPassword2 = "smtptest";
>  
> +add_task(async function () {

I would drop the space before (). Or maybe these anonymous functions should get a name to be shown when they fail?

::: mailnews/compose/test/unit/test_smtpPassword.js
@@ +16,5 @@
>  // Password needs to match the login information stored in the signons json
>  // file.
>  var kPassword = "smtptest";
>  
> +add_task(async function () {

Here too.

@@ +29,5 @@
>    }
>    server = setupServerDaemon(createHandler);
>  
>    // Prepare files for passwords (generated by a script in bug 1018624).
> +  await setupForPassword("signons-mailnews1.8.json")

Missing ; (not your fault).

::: mailnews/compose/test/unit/test_smtpPassword2.js
@@ +12,5 @@
>  var kProtocol = "smtp";
>  var kHostname = "localhost";
>  var kServerUrl = kProtocol + "://" + kHostname;
>  
> +add_task(async function () {

Surplus space.

@@ +17,2 @@
>    // Prepare files for passwords (generated by a script in bug 1018624).
> +  await setupForPassword("signons-mailnews1.8-multiple.json")

Missing ; (not your fault).

::: mailnews/compose/test/unit/test_smtpPasswordFailure1.js
@@ +55,5 @@
>        return 1;
>    }
>  }
>  
> +add_task(async function () {

Space

@@ +68,5 @@
>    }
>    server = setupServerDaemon(createHandler);
>  
>    // Prepare files for passwords (generated by a script in bug 1018624).
> +  await setupForPassword("signons-mailnews1.8.json")

Missing ; (not your fault).

::: mailnews/compose/test/unit/test_smtpPasswordFailure2.js
@@ +65,5 @@
>    }
>    return false;
>  }
>  
> +add_task(async function () {

Space

@@ +79,5 @@
>    }
>    server = setupServerDaemon(createHandler);
>  
>    // Prepare files for passwords (generated by a script in bug 1018624).
> +  await setupForPassword("signons-mailnews1.8.json")

Missing ; (not your fault).

::: mailnews/compose/test/unit/test_smtpPasswordFailure3.js
@@ +63,5 @@
>    }
>    return false;
>  }
>  
> +add_task(async function () {

Space

@@ +78,5 @@
>    }
>    server = setupServerDaemon(createHandler);
>  
>    // Prepare files for passwords (generated by a script in bug 1018624).
> +  await setupForPassword("signons-mailnews1.8.json")

Missing ; (not your fault).

::: mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js
@@ +216,5 @@
>  
>  // basic preparation done for each test
>  function setupTest(aFilter, aAction)
>  {
> +  return Task.spawn(async function () {

Space

::: mailnews/imap/test/unit/test_imapPasswordFailure.js
@@ +56,5 @@
>    }
>    return false;
>  }
>  
> +add_task(async function () {

Space

@@ +61,4 @@
>    do_test_pending();
>  
>    // Prepare files for passwords (generated by a script in bug 1018624).
> +  await setupForPassword("signons-mailnews1.8-imap.json")

Missing ; (not your fault).

::: mailnews/local/test/unit/test_movemailDownload.js
@@ +80,5 @@
>  
>    run_next_test();
>  }
>  
> +var streamNextMessage = async function (aMsgHdr) {

Space

::: mailnews/local/test/unit/test_pop3Password.js
@@ +104,5 @@
>        thread.processNextEvent(true);
>    }
>  }
>  
> +add_task(async function () {

Space

::: mailnews/local/test/unit/test_pop3Password2.js
@@ +106,5 @@
>        thread.processNextEvent(true);
>    }
>  }
>  
> +add_task(async function () {

Space

::: mailnews/local/test/unit/test_pop3Password3.js
@@ +13,5 @@
>  var kProtocol = "pop3";
>  var kHostname = "localhost";
>  var kServerUrl = "mailbox://" + kHostname;
>  
> +add_task(async function () {

Space

::: mailnews/local/test/unit/test_pop3PasswordFailure.js
@@ +173,5 @@
>  {
>    do_test_finished();
>  }
>  
> +add_task(async function () {

Space

::: mailnews/news/test/unit/test_nntpGroupPassword.js
@@ +8,5 @@
>  
>  // Define these up here for checking with the transaction
>  var test = null;
>  
> +add_task(async function () {

Space

::: mailnews/news/test/unit/test_nntpPassword.js
@@ +13,5 @@
>  
>  // Define these up here for checking with the transaction
>  var test = null;
>  
> +add_task(async function () {

Space

::: mailnews/news/test/unit/test_nntpPassword2.js
@@ +16,5 @@
>  
>  // Define these up here for checking with the transaction
>  var test = null;
>  
> +add_task(async function () {

Space

::: mailnews/news/test/unit/test_nntpPassword3.js
@@ +13,5 @@
>  var kProtocol = "nntp";
>  var kHostname = "localhost";
>  var kServerUrl = "news://" + kHostname;
>  
> +add_task(async function () {

Space

@@ +18,2 @@
>    // Prepare files for passwords (generated by a script in bug 1018624).
> +  await setupForPassword("signons-mailnews1.8.json")

Missing ; (not your fault).

Comment 34

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #33)
> > +add_task(async function () {
> I would drop the space before (). Or maybe these anonymous functions should
> get a name to be shown when they fail?
I believe the standard is:
function () {...} and function f() {...}. So I adhere to this standard:
https://searchfox.org/mozilla-central/search?q=async+function+()+%7B&case=false&regexp=false&path=

Sorry, I have on intention to add the missing semicolons.

Comment 35

[Jorg K (CEST = GMT+2)]

Attachment: 1455536-async-spawn.patch

Sorry about the poor quality patch with the surplus ")" :-(

I hope this is better. I ran the tests manually and they pass.

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/0a4b7367928f
Follow-up: Remove remaining unneeded uses of Task.{async|spawn}. r=arai