Bug 1851536 - Await `TestUtils.promiseTestFinished` instead of passing test scope around. r=mkmelin

This is much cleaner than passing the test scope to each module and calling `registerCleanupFunction`
to shut down servers etc..

`TestUtils.promiseTestFinished` isn't called in XPCShell tests but because they run separately and
everything is cleaned up when the process ends, that doesn't matter.

Differential Revision: https://phabricator.services.mozilla.com/D202679

--HG--
extra : rebase_source : 4d3fe3ee0e32777734ccb850c30756cd4074c886
extra : amend_source : 9286c4162da1985f2fa9f7c50198501aecdac011

calendar/test/browser/browser_calDAV_oAuth.js

@@ -36,7 +36,7 @@ const googleLogin = { ...defaultLogin, scope: GOOGLE_SCOPE };
 
 add_setup(async function () {
   Services.logins.removeAllLogins();
-  await OAuth2TestUtils.startServer(this);
+  await OAuth2TestUtils.startServer();
 });
 
 /**

mail/base/test/IMAPServer.sys.mjs

@@ -5,6 +5,7 @@
 import * as ImapD from "resource://testing-common/mailnews/Imapd.sys.mjs";
 import { mailTestUtils } from "resource://testing-common/mailnews/MailTestUtils.sys.mjs";
 import { nsMailServer } from "resource://testing-common/mailnews/Maild.sys.mjs";
+import { TestUtils } from "resource://testing-common/TestUtils.sys.mjs";
 
 const { IMAP_RFC3501_handler, ImapDaemon, ImapMessage, mixinExtension } = ImapD;
 
@@ -12,8 +13,7 @@ const { IMAP_RFC3501_handler, ImapDaemon, ImapMessage, mixinExtension } = ImapD;
  * A simple IMAP server for testing purposes.
  */
 export class IMAPServer {
-  constructor(testScope, options = {}) {
-    this.testScope = testScope;
+  constructor(options = {}) {
     this.options = options;
     this.open(options.extensions);
   }
@@ -37,7 +37,7 @@ export class IMAPServer {
     this.server.start();
     dump(`IMAP server at localhost:${this.server.port} opened\n`);
 
-    this.testScope.registerCleanupFunction(() => {
+    TestUtils.promiseTestFinished?.then(() => {
       this.close();
       dump(`IMAP server at localhost:${this.server.port} closed\n`);
     });

mail/base/test/NNTPServer.sys.mjs

@@ -10,13 +10,13 @@ import {
   NNTP_RFC4643_extension,
   NntpDaemon,
 } from "resource://testing-common/mailnews/Nntpd.sys.mjs";
+import { TestUtils } from "resource://testing-common/TestUtils.sys.mjs";
 
 /**
  * A simple NNTP server for testing purposes.
  */
 export class NNTPServer {
-  constructor(testScope, options = {}) {
-    this.testScope = testScope;
+  constructor(options = {}) {
     this.options = options;
     this.open();
   }
@@ -33,7 +33,7 @@ export class NNTPServer {
     this.server.start();
     dump(`NNTP server at localhost:${this.server.port} opened\n`);
 
-    this.testScope.registerCleanupFunction(() => {
+    TestUtils.promiseTestFinished?.then(() => {
       this.close();
       dump(`NNTP server at localhost:${this.server.port} closed\n`);
     });

mail/base/test/POP3Server.sys.mjs

@@ -4,13 +4,13 @@
 
 import * as Pop3D from "resource://testing-common/mailnews/Pop3d.sys.mjs";
 import { nsMailServer } from "resource://testing-common/mailnews/Maild.sys.mjs";
+import { TestUtils } from "resource://testing-common/TestUtils.sys.mjs";
 
 /**
  * A simple POP3 server for testing purposes.
  */
 export class POP3Server {
-  constructor(testScope, options = {}) {
-    this.testScope = testScope;
+  constructor(options = {}) {
     this.options = options;
     this.open(options.handler);
   }
@@ -34,7 +34,7 @@ export class POP3Server {
     this.server.start();
     dump(`POP3 server at localhost:${this.server.port} opened\n`);
 
-    this.testScope.registerCleanupFunction(() => {
+    TestUtils.promiseTestFinished?.then(() => {
       this.close();
       dump(`POP3 server at localhost:${this.server.port} closed\n`);
     });

mail/base/test/SMTPServer.sys.mjs

@@ -7,13 +7,13 @@ import {
   SMTP_RFC2821_handler,
 } from "resource://testing-common/mailnews/Smtpd.sys.mjs";
 import { nsMailServer } from "resource://testing-common/mailnews/Maild.sys.mjs";
+import { TestUtils } from "resource://testing-common/TestUtils.sys.mjs";
 
 /**
  * A simple SMTP server for testing purposes.
  */
 export class SMTPServer {
-  constructor(testScope, options = {}) {
-    this.testScope = testScope;
+  constructor(options = {}) {
     this.options = options;
     this.open();
   }
@@ -34,7 +34,7 @@ export class SMTPServer {
     this.server.start();
     dump(`SMTP server at localhost:${this.server.port} opened\n`);
 
-    this.testScope.registerCleanupFunction(() => {
+    TestUtils.promiseTestFinished?.then(() => {
       this.close();
       dump(`SMTP server at localhost:${this.server.port} closed\n`);
     });

mail/base/test/browser/browser_editMenu.js

@@ -79,7 +79,7 @@ add_setup(async function () {
   folderInfo.setCharProperty("searchStr", "ALL");
   folderInfo.setCharProperty("searchFolderUri", testFolder.URI);
 
-  nntpServer = new NNTPServer(this);
+  nntpServer = new NNTPServer();
   nntpServer.addGroup("edit.menu.newsgroup");
   const nntpAccount = MailServices.accounts.createAccount();
   nntpAccount.incomingServer = MailServices.accounts.createIncomingServer(
@@ -92,7 +92,7 @@ add_setup(async function () {
   nntpRootFolder.createSubfolder("edit.menu.newsgroup", null);
   nntpFolder = nntpRootFolder.getChildNamed("edit.menu.newsgroup");
 
-  imapServer = new IMAPServer(this);
+  imapServer = new IMAPServer();
   const imapAccount = MailServices.accounts.createAccount();
   imapAccount.addIdentity(MailServices.accounts.createIdentity());
   imapAccount.incomingServer = MailServices.accounts.createIncomingServer(

mail/base/test/browser/browser_folderPaneContext.js

@@ -108,7 +108,7 @@ add_setup(async function () {
   folderInfo.setCharProperty("searchStr", "ALL");
   folderInfo.setCharProperty("searchFolderUri", plainFolder.URI);
 
-  nntpServer = new NNTPServer(this);
+  nntpServer = new NNTPServer();
   nntpServer.addGroup("folder.pane.context.newsgroup");
   nntpServer.addMessages(
     "folder.pane.context.newsgroup",

mail/base/test/browser/browser_getMessages.js

@@ -48,14 +48,11 @@ add_setup(async function () {
   localAccount = MailServices.accounts.createLocalMailAccount();
   localRootFolder = localAccount.incomingServer.rootFolder;
 
-  [imapServer, pop3Server, nntpServer] = await ServerTestUtils.createServers(
-    this,
-    [
-      ServerTestUtils.serverDefs.imap.plain,
-      ServerTestUtils.serverDefs.pop3.plain,
-      ServerTestUtils.serverDefs.nntp.plain,
-    ]
-  );
+  [imapServer, pop3Server, nntpServer] = await ServerTestUtils.createServers([
+    ServerTestUtils.serverDefs.imap.plain,
+    ServerTestUtils.serverDefs.pop3.plain,
+    ServerTestUtils.serverDefs.nntp.plain,
+  ]);
   nntpServer.addGroup("getmessages.newsgroup");
 
   imapAccount = MailServices.accounts.createAccount();

mail/base/test/browser/browser_getMessages_badPassword.js

@@ -36,17 +36,14 @@ add_setup(async function () {
   localAccount = MailServices.accounts.createLocalMailAccount();
   localRootFolder = localAccount.incomingServer.rootFolder;
 
-  [imapServer, pop3Server, nntpServer] = await ServerTestUtils.createServers(
-    this,
-    [
-      ServerTestUtils.serverDefs.imap.plain,
-      ServerTestUtils.serverDefs.pop3.plain,
-      {
-        ...ServerTestUtils.serverDefs.nntp.plain,
-        options: { username: "user", password: "password" },
-      },
-    ]
-  );
+  [imapServer, pop3Server, nntpServer] = await ServerTestUtils.createServers([
+    ServerTestUtils.serverDefs.imap.plain,
+    ServerTestUtils.serverDefs.pop3.plain,
+    {
+      ...ServerTestUtils.serverDefs.nntp.plain,
+      options: { username: "user", password: "password" },
+    },
+  ]);
   nntpServer.addGroup("getmessages.newsgroup");
 
   imapAccount = MailServices.accounts.createAccount();

mail/base/test/browser/browser_getMessages_certError.js

@@ -42,7 +42,7 @@ add_setup(async function () {
   localAccount = MailServices.accounts.createLocalMailAccount();
   localRootFolder = localAccount.incomingServer.rootFolder;
 
-  [imapServer, pop3Server] = await ServerTestUtils.createServers(this, [
+  [imapServer, pop3Server] = await ServerTestUtils.createServers([
     ServerTestUtils.serverDefs.imap.expiredTLS,
     ServerTestUtils.serverDefs.pop3.expiredTLS,
   ]);

mail/base/test/browser/browser_getMessages_deferredAccount.js

@@ -39,7 +39,7 @@ add_setup(async function () {
     .QueryInterface(Ci.nsIMsgLocalMailFolder)
     .createLocalSubfolder("Inbox");
 
-  pop3Server = await createServer(this, serverDefs.pop3.plain);
+  pop3Server = await createServer(serverDefs.pop3.plain);
 
   pop3Account = MailServices.accounts.createAccount();
   pop3Account.addIdentity(MailServices.accounts.createIdentity());

mail/base/test/browser/browser_getMessages_oAuth2.js

@@ -39,7 +39,7 @@ add_setup(async function () {
   localAccount = MailServices.accounts.createLocalMailAccount();
   localRootFolder = localAccount.incomingServer.rootFolder;
 
-  [imapServer, pop3Server] = await ServerTestUtils.createServers(this, [
+  [imapServer, pop3Server] = await ServerTestUtils.createServers([
     ServerTestUtils.serverDefs.imap.oAuth,
     ServerTestUtils.serverDefs.pop3.oAuth,
   ]);

mail/base/test/browser/browser_getMessages_offline.js

@@ -38,14 +38,11 @@ add_setup(async function () {
   localAccount = MailServices.accounts.createLocalMailAccount();
   localRootFolder = localAccount.incomingServer.rootFolder;
 
-  [imapServer, pop3Server, nntpServer] = await ServerTestUtils.createServers(
-    this,
-    [
-      ServerTestUtils.serverDefs.imap.plain,
-      ServerTestUtils.serverDefs.pop3.plain,
-      ServerTestUtils.serverDefs.nntp.plain,
-    ]
-  );
+  [imapServer, pop3Server, nntpServer] = await ServerTestUtils.createServers([
+    ServerTestUtils.serverDefs.imap.plain,
+    ServerTestUtils.serverDefs.pop3.plain,
+    ServerTestUtils.serverDefs.nntp.plain,
+  ]);
   nntpServer.addGroup("getmessages.offline");
 
   imapAccount = MailServices.accounts.createAccount();

mail/base/test/browser/browser_mailContext_compose.js

@@ -65,7 +65,7 @@ add_setup(async function () {
   templatesFolder.addMessage(generator.makeMessage().toMessageString());
   templateMessage = templatesFolder.messages.getNext();
 
-  const nntpServer = new NNTPServer(this);
+  const nntpServer = new NNTPServer();
   nntpServer.addGroup("mailContext.compose");
   nntpServer.addMessages("mailContext.compose", [generator.makeMessage()]);
   const nntpAccount = MailServices.accounts.createAccount();

mail/base/test/browser/browser_markAsRead.js

@@ -35,7 +35,7 @@ add_setup(async function () {
     generator.makeMessages({}).map(message => message.toMessageString())
   );
 
-  const imapServer = new IMAPServer(this);
+  const imapServer = new IMAPServer();
   const imapAccount = MailServices.accounts.createAccount();
   imapAccount.addIdentity(MailServices.accounts.createIdentity());
   imapAccount.incomingServer = MailServices.accounts.createIncomingServer(

mail/base/test/browser/browser_subscribe.js

@@ -20,7 +20,7 @@ let nntpRootFolder;
 
 add_setup(async function () {
   const imapAccount = MailServices.accounts.createAccount();
-  imapServer = new IMAPServer(this, { username: `${imapAccount.key}user` });
+  imapServer = new IMAPServer({ username: `${imapAccount.key}user` });
   imapServer.daemon.createMailbox("Bar", { subscribed: false });
   imapServer.daemon.createMailbox("Baz", { subscribed: true });
   imapServer.daemon.createMailbox("Foo", { subscribed: false });
@@ -39,7 +39,7 @@ add_setup(async function () {
   imapAccount.incomingServer.deleteModel = Ci.nsMsgImapDeleteModels.IMAPDelete;
   imapRootFolder = imapAccount.incomingServer.rootFolder;
 
-  nntpServer = new NNTPServer(this);
+  nntpServer = new NNTPServer();
   nntpServer.addGroup("subscribe.bar");
   nntpServer.addGroup("subscribe.baz");
   nntpServer.addGroup("subscribe.baz.subbaz");

mail/components/accountcreation/test/browser/browser_verifyConfig.js

@@ -35,7 +35,7 @@ add_setup(async function () {
   tlsCert = await getCertificate("valid");
   expiredCert = await getCertificate("expired");
 
-  await createServers(this, [
+  await createServers([
     serverDefs.imap.tls,
     serverDefs.imap.expiredTLS,
     serverDefs.pop3.tls,

mail/components/accountcreation/test/xpcshell/test_guessConfig.js

@@ -39,7 +39,7 @@ add_setup(async function () {
   tlsCert = await getCertificate("valid");
   expiredCert = await getCertificate("expired");
 
-  await createServers(this, [
+  await createServers([
     { ...serverDefs.imap.plain, aliases: [["alt.test.test", 143]] },
     serverDefs.imap.startTLS,
     {
@@ -778,7 +778,7 @@ add_task(async function testGuessConfig() {
  * the base domain.
  */
 add_task(async function testGuessConfigKnownSubdomains() {
-  const [imapServer, pop3Server, smtpServer] = await createServers(this, [
+  const [imapServer, pop3Server, smtpServer] = await createServers([
     { ...serverDefs.imap.tls, hostname: "imap.test.test" },
     { ...serverDefs.pop3.tls, hostname: "pop3.test.test" },
     { ...serverDefs.smtp.tls, hostname: "smtp.test.test" },

mail/components/addrbook/test/browser/browser_cardDAV_oAuth.js

@@ -37,7 +37,7 @@ const PATH = "comm/mail/components/addrbook/test/browser/data/";
 const URL = `http://mochi.test:8888/browser/${PATH}`;
 
 add_setup(async function () {
-  await OAuth2TestUtils.startServer(this);
+  await OAuth2TestUtils.startServer();
 });
 
 /**

mailnews/test/resources/HttpsProxy.sys.mjs

@@ -8,6 +8,7 @@
 
 import { CommonUtils } from "resource://services-common/utils.sys.mjs";
 import { NetUtil } from "resource://gre/modules/NetUtil.sys.mjs";
+import { TestUtils } from "resource://testing-common/TestUtils.sys.mjs";
 
 const socketTransportService = Cc[
   "@mozilla.org/network/socket-transport-service;1"
@@ -25,13 +26,11 @@ export class HttpsProxy {
   handlers = [];
 
   /**
-   * @param {object} testScope - The JS scope for the current test, so
-   *   `registerCleanupFunction` can be used.
    * @param {integer} serverPort - The port number of the HTTP server.
    * @param {nsIX509Cert} tlsCert - The certificate to use for HTTPS requests.
    *   See ServerTestUtils.getCertificate.
    */
-  constructor(testScope, serverPort, tlsCert) {
+  constructor(serverPort, tlsCert) {
     this.serverPort = serverPort;
 
     this.socket = Cc["@mozilla.org/network/tls-server-socket;1"].createInstance(
@@ -45,7 +44,7 @@ export class HttpsProxy {
       `Reverse proxy from localhost:${this.serverPort} to localhost:${this.socket.port} opened\n`
     );
 
-    testScope.registerCleanupFunction(() => {
+    TestUtils.promiseTestFinished?.then(() => {
       this.close();
       dump(
         `Reverse proxy from localhost:${this.serverPort} to localhost:${this.socket.port} closed\n`

mailnews/test/resources/NetworkTestUtils.sys.mjs

@@ -10,6 +10,7 @@
 var CC = Components.Constructor;
 
 import { NetUtil } from "resource://gre/modules/NetUtil.sys.mjs";
+import { TestUtils } from "resource://testing-common/TestUtils.sys.mjs";
 
 const ServerSocket = CC(
   "@mozilla.org/network/server-socket;1",
@@ -265,6 +266,10 @@ export var NetworkTestUtils = {
       }`;
       Services.prefs.setIntPref("network.proxy.type", 2);
       Services.prefs.setCharPref("network.proxy.autoconfig_url", pac);
+
+      TestUtils.promiseTestFinished?.then(() => {
+        this.clearProxy();
+      });
     }
 
     dump("Requesting to map " + hostname + ":" + port + "\n");

mailnews/test/resources/OAuth2TestUtils.sys.mjs

@@ -12,6 +12,7 @@ import { HttpsProxy } from "resource://testing-common/mailnews/HttpsProxy.sys.mj
 import { HttpServer, HTTP_405 } from "resource://testing-common/httpd.sys.mjs";
 import { NetworkTestUtils } from "resource://testing-common/mailnews/NetworkTestUtils.sys.mjs";
 import { ServerTestUtils } from "resource://testing-common/mailnews/ServerTestUtils.sys.mjs";
+import { TestUtils } from "resource://testing-common/TestUtils.sys.mjs";
 
 const { OAuth2Module } = ChromeUtils.import(
   "resource:///modules/OAuth2Module.jsm"
@@ -22,21 +23,14 @@ const validCodes = new Set();
 export const OAuth2TestUtils = {
   /**
    * Start an OAuth2 server and add it to the proxy at oauth.test.test:443.
-   *
-   * @param {object} testScope - The JS scope for the current test, so
-   *   `registerCleanupFunction` can be used.
    */
-  async startServer(testScope) {
-    const oAuth2Server = new OAuth2Server(testScope);
+  async startServer() {
+    const oAuth2Server = new OAuth2Server();
     const httpsProxy = new HttpsProxy(
-      testScope,
       oAuth2Server.httpServer.identity.primaryPort,
       await ServerTestUtils.getCertificate("oauth")
     );
     NetworkTestUtils.configureProxy("oauth.test.test", 443, httpsProxy.port);
-    testScope.registerCleanupFunction(() => {
-      NetworkTestUtils.clearProxy();
-    });
     return oAuth2Server;
   },
 
@@ -133,7 +127,7 @@ class OAuth2Server {
   refreshToken = "refresh_token";
   expiry = null;
 
-  constructor(testScope) {
+  constructor() {
     this.httpServer = new HttpServer();
     this.httpServer.registerPathHandler("/form", this.formHandler.bind(this));
     this.httpServer.registerPathHandler(
@@ -146,7 +140,7 @@ class OAuth2Server {
     const port = this.httpServer.identity.primaryPort;
     dump(`OAuth2 server at localhost:${port} opened\n`);
 
-    testScope.registerCleanupFunction(() => {
+    TestUtils.promiseTestFinished?.then(() => {
       this.httpServer.stop();
       dump(`OAuth2 server at localhost:${port} closed\n`);
     });

mailnews/test/resources/ServerTestUtils.sys.mjs

@@ -23,8 +23,6 @@ const serverConstructors = {
 // Change this for more server debugging output. See Maild.sys.mjs for values.
 const serverDebugLevel = 0;
 
-let cleanupFunctionRegistered = false;
-
 /**
  * @typedef ServerDef
  * @property {"imap"|"pop3"|"smtp"} type - What type of server do we want?
@@ -41,49 +39,43 @@ let cleanupFunctionRegistered = false;
 /**
  * Create and start a single server.
  *
- * @param {object} testScope - The environment in which the test is running.
  * @param {ServerDef} def - The server definition.
  * @returns {IMAPServer|POP3Server|SMTPServer}
  */
-async function createServer(
-  testScope,
-  { type, baseOptions = {}, options = {}, hostname, port, aliases = [] }
-) {
+async function createServer({
+  type,
+  baseOptions = {},
+  options = {},
+  hostname,
+  port,
+  aliases = [],
+}) {
   options = { ...baseOptions, ...options };
   if (options.tlsCertFile && !options.tlsCert) {
     options.tlsCert = await getCertificate(options.tlsCertFile);
   }
 
-  const server = new serverConstructors[type](testScope, options);
+  const server = new serverConstructors[type](options);
   server.server.setDebugLevel(serverDebugLevel);
   NetworkTestUtils.configureProxy(hostname, port, server.port);
   for (const [aliasHostname, aliasPort] of aliases) {
     NetworkTestUtils.configureProxy(aliasHostname, aliasPort, server.port);
   }
 
-  if (!cleanupFunctionRegistered) {
-    testScope.registerCleanupFunction(function () {
-      NetworkTestUtils.clearProxy();
-      cleanupFunctionRegistered = false;
-    });
-    cleanupFunctionRegistered = true;
-  }
-
   return server;
 }
 
 /**
  * Create and start multiple servers.
  *
- * @param {object} testScope - The environment in which the test is running.
  * @param {ServerDef[]} serverDefs - The server definitions.
  * @returns {IMAPServer[]|POP3Server[]|SMTPServer[]} - The created servers,
  *   in the same order as the definitions given.
  */
-async function createServers(testScope, serverDefs) {
+async function createServers(serverDefs) {
   const servers = [];
   for (const serverDef of serverDefs) {
-    servers.push(await createServer(testScope, serverDef));
+    servers.push(await createServer(serverDef));
   }
   return servers;
 }