Closed Bug 1455536 Opened Last year Closed Last year

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

Categories

(Thunderbird :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 61.0

People

(Reporter: jorgk, Assigned: jorgk)

Details

Attachments

(5 files, 8 obsolete files)

19.86 KB, patch
Fallen
: review+
Details | Diff | Splinter Review
16.43 KB, patch
jorgk
: review+
Details | Diff | Splinter Review
5.00 KB, patch
jorgk
: review+
Details | Diff | Splinter Review
97.48 KB, patch
aceman
: review+
Details | Diff | Splinter Review
8.38 KB, patch
arai
: review+
Details | Diff | Splinter Review
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".
Summary: Massive Xpcshell test failure on 2018-04-19: xx failing tests → Massive Xpcshell test failure on 2018-04-19: 100 failing tests - caused by task not allowed to be generators
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?
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(florian)
I guess in calendar might not run into the same problems, so let's see what that gives.
Attachment #8969597 - Flags: review?(philipp)
I'll try it when my local build is finished.
Attachment #8969599 - Flags: review?(florian)
Just great. I thought generators were the new trend? Are they just not allowed in some "task" context? What is it?
Flags: needinfo?(arai.unmht)
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}]"
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 :-(
Keywords: leave-open
Pushed by mozilla@jorgk.com:
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
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.
Summary: Massive Xpcshell test failure on 2018-04-19: 100 failing tests - caused by task not allowed to be generators → Massive Xpcshell test failure on 2018-04-19: 47 failing tests (41 mailnews, 5 calendar, 1 chat) - caused by task not allowed to be generators
Summary: Massive Xpcshell test failure on 2018-04-19: 47 failing tests (41 mailnews, 5 calendar, 1 chat) - caused by task not allowed to be generators → 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
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.
Attachment #8969639 - Attachment is obsolete: true
Flags: needinfo?(arai.unmht)
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.
Attachment #8969599 - Flags: review?(florian) → review-
(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.
Flags: needinfo?(florian)
I don't see that "unrelated failure any more" in the latest try run.
Flags: needinfo?(kmaglione+bmo)
(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.
Removing Task.async() here as well as per Florian's review for chat.
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=94504f3356380651407d04df944f489bec7fc357
Attached patch 1455536-remove-task-async.patch (obsolete) — Splinter Review
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.
Attachment #8969732 - Flags: review?(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) {
Attachment #8969732 - Flags: review?(florian) → review+
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
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.
Switched to async processLog(aLog).
Attachment #8969732 - Attachment is obsolete: true
Attachment #8969742 - Flags: review+
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.
Keywords: leave-open
OK, going back to v3.
Attachment #8969721 - Attachment is obsolete: true
Attachment #8969723 - Attachment is obsolete: true
Attachment #8969726 - Attachment is obsolete: true
Attachment #8969750 - Flags: review?(acelists)
Pushed by mozilla@jorgk.com:
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
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
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?
Assignee: nobody → jorgk
Flags: needinfo?(acelists)
Target Milestone: --- → Thunderbird 61.0
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.
Attachment #8969599 - Flags: review- → review+
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 () {|?
Flags: needinfo?(florian)
just `(async function() { ... })()`
OK, I'll do a patch.
Flags: needinfo?(florian)
Flags: needinfo?(acelists)
Attached patch 1455536-async-spawn.patch (obsolete) — Splinter Review
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.
Attachment #8970084 - Flags: review?(florian)
Attachment #8970084 - Flags: review?(arai.unmht)
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();
Attachment #8970084 - Flags: review?(arai.unmht) → review-
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).
Attachment #8969750 - Flags: review?(acelists) → review+
Attachment #8970084 - Flags: review?(florian)
(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.
Sorry about the poor quality patch with the surplus ")" :-(

I hope this is better. I ran the tests manually and they pass.
Attachment #8970084 - Attachment is obsolete: true
Attachment #8970109 - Flags: review?(arai.unmht)
Attachment #8970109 - Flags: review?(arai.unmht) → review+
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/0a4b7367928f
Follow-up: Remove remaining unneeded uses of Task.{async|spawn}. r=arai
Attachment #8969597 - Flags: review?(philipp) → review+
You need to log in before you can comment on or make changes to this bug.