Improved network access and mocking in tests (#16371)

refs https://github.com/TryGhost/Team/issues/2667

Some tests still accessed the internet. Now network access is disabled
by default. This change also introduces two helper methods related to
networking (mocking Slack and Mailgun).

This fixes two unreliable tests:
- Staff service was accessing a Slack test API -> timeout possible
- MentionSendingService was trying to send webmentions for every post
publish/change -> possible timeouts and job issues

ghost/core/test/e2e-api/admin/oembed.test.js

@@ -5,6 +5,7 @@ const supertest = require('supertest');
 const testUtils = require('../../utils/index');
 const config = require('../../../core/shared/config/index');
 const localUtils = require('./utils');
+const {mockManager} = require('../../utils/e2e-framework');
 
 // for sinon stubs
 const dnsPromises = require('dns').promises;
@@ -20,9 +21,7 @@ describe('Oembed API', function () {
 
     beforeEach(function () {
         // ensure sure we're not network dependent
-        sinon.stub(dnsPromises, 'lookup').callsFake(function () {
+        mockManager.disableNetwork();
-            return Promise.resolve({address: '123.123.123.123'});
-        });
     });
 
     afterEach(function () {

ghost/core/test/e2e-server/services/mentions.test.js

@@ -1,9 +1,7 @@
 const {agentProvider, fixtureManager, mockManager} = require('../../utils/e2e-framework');
-const sinon = require('sinon');
 const nock = require('nock');
 const assert = require('assert');
 const markdownToMobiledoc = require('../../utils/fixtures/data-generator').markdownToMobiledoc;
-const dnsPromises = require('dns').promises;
 const jobsService = require('../../../core/server/services/mentions-jobs');
 
 let agent;
@@ -30,9 +28,7 @@ describe('Mentions Service', function () {
 
     beforeEach(async function () {
         // externalRequest does dns lookup; stub to make sure we don't fail with fake domain names
-        sinon.stub(dnsPromises, 'lookup').callsFake(function () {
+        mockManager.disableNetwork();
-            return Promise.resolve({address: '123.123.123.123'});
-        });
 
         // mock response from website mentioned by post to provide endpoint
         mentionMock = nock(mentionUrl.href)

ghost/core/test/integration/services/email-service/batch-sending.test.js

@@ -181,9 +181,14 @@ describe('Batch sending tests', function () {
         stubbedSend = sinon.fake.resolves({
             id: 'stubbed-email-id'
         });
+        mockManager.mockMail();
+        mockManager.mockMailgun(function () {
+            return stubbedSend.call(this, ...arguments);
+        });
     });
 
     afterEach(async function () {
+        mockManager.restore();
         await configUtils.restore();
         await models.Settings.edit([{
             key: 'email_verification_required',
@@ -192,21 +197,6 @@ describe('Batch sending tests', function () {
     });
 
     before(async function () {
-        mockManager.mockSetting('mailgun_api_key', 'test');
-        mockManager.mockSetting('mailgun_domain', 'example.com');
-        mockManager.mockSetting('mailgun_base_url', 'test');
-        mockManager.mockMail();
-
-        // We need to stub the Mailgun client before starting Ghost
-        sinon.stub(MailgunClient.prototype, 'getInstance').returns({
-            // @ts-ignore
-            messages: {
-                create: async function () {
-                    return await stubbedSend.call(this, ...arguments);
-                }
-            }
-        });
-
         const agents = await agentProvider.getAgentsWithFrontend();
         agent = agents.adminAgent;
         frontendAgent = agents.frontendAgent;
@@ -223,7 +213,6 @@ describe('Batch sending tests', function () {
     });
 
     after(async function () {
-        mockManager.restore();
         await ghostServer.stop();
     });
 
@@ -240,7 +229,7 @@ describe('Batch sending tests', function () {
         await completedPromise;
 
         await emailModel.refresh();
-        assert.equal(emailModel.get('status'), 'submitted');
+        assert.equal(emailModel.get('status'), 'submitted', 'This email should be in submitted state: ' + (emailModel.get('error') ?? 'No error'));
         assert.equal(emailModel.get('email_count'), 4);
 
         // Did we create batches?

ghost/core/test/unit/server/services/oembed/twitter-embed.test.js

@@ -3,6 +3,7 @@ const TwitterOEmbedProvider = require('../../../../../core/server/services/oembe
 const externalRequest = require('../../../../../core/server/lib/request-external');
 const nock = require('nock');
 const sinon = require('sinon');
+const {mockManager} = require('../../../../utils/e2e-framework');
 const dnsPromises = require('dns').promises;
 
 describe('TwitterOEmbedProvider', function () {
@@ -12,14 +13,11 @@ describe('TwitterOEmbedProvider', function () {
 
     beforeEach(function () {
         // external requests will attempt dns lookup
-        sinon.stub(dnsPromises, 'lookup').callsFake(function () {
+        mockManager.disableNetwork();
-            return Promise.resolve({address: '123.123.123.123'});
-        });
     });
 
     afterEach(async function () {
-        nock.cleanAll();
+        mockManager.restore();
-        sinon.restore();
     });
 
     after(async function () {

ghost/core/test/unit/server/services/staff/index.test.js

@@ -18,6 +18,7 @@ describe('Staff Service:', function () {
 
     beforeEach(function () {
         mockManager.mockMail();
+        mockManager.mockSlack();
         userModelStub = sinon.stub(models.User, 'getEmailAlertUsers').resolves([{
             email: 'owner@ghost.org',
             slug: 'ghost'

ghost/core/test/utils/e2e-framework-mock-manager.js

@@ -2,6 +2,7 @@ const errors = require('@tryghost/errors');
 const sinon = require('sinon');
 const assert = require('assert');
 const nock = require('nock');
+const MailgunClient = require('@tryghost/mailgun-client/lib/mailgun-client');
 
 // Helper services
 const configUtils = require('./configUtils');
@@ -35,17 +36,31 @@ const disableStripe = async () => {
     await stripeService.disconnect();
 };
 
-const mockStripe = () => {
-    nock.disableNetConnect();
-};
-
 const disableNetwork = () => {
     nock.disableNetConnect();
 
     // externalRequest does dns lookup; stub to make sure we don't fail with fake domain names
+    if (!dnsPromises.lookup.restore) {
         sinon.stub(dnsPromises, 'lookup').callsFake(() => {
             return Promise.resolve({address: '123.123.123.123', family: 4});
         });
+    }
+
+    // Allow localhost
+    nock.enableNetConnect('127.0.0.1');
+};
+
+const mockStripe = () => {
+    disableNetwork();
+};
+
+const mockSlack = () => {
+    disableNetwork();
+
+    nock(/hooks.slack.com/)
+        .persist()
+        .post('/')
+        .reply(200, 'ok');
 };
 
 /**
@@ -68,6 +83,26 @@ const mockMail = (response = 'Mail is disabled') => {
     return mockMailReceiver;
 };
 
+const mockMailgun = (customStubbedSend) => {
+    mockSetting('mailgun_api_key', 'test');
+    mockSetting('mailgun_domain', 'example.com');
+    mockSetting('mailgun_base_url', 'test');
+
+    const stubbedSend = customStubbedSend ?? sinon.fake.resolves({
+        id: `<${new Date().getTime()}.${0}.5817@samples.mailgun.org>`
+    });
+
+    // We need to stub the Mailgun client before starting Ghost
+    sinon.stub(MailgunClient.prototype, 'getInstance').returns({
+        // @ts-ignore
+        messages: {
+            create: async function () {
+                return await stubbedSend.call(this, ...arguments);
+            }
+        }
+    });
+};
+
 const mockWebhookRequests = () => {
     mocks.webhookMockReceiver = new WebhookMockReceiver({snapshotManager});
 
@@ -210,6 +245,9 @@ const restore = () => {
     }
 
     mailService.GhostMailer.prototype.send = originalMailServiceSend;
+
+    // Disable network again after restoring sinon
+    disableNetwork();
 };
 
 module.exports = {
@@ -217,6 +255,8 @@ module.exports = {
     mockMail,
     disableStripe,
     mockStripe,
+    mockSlack,
+    mockMailgun,
     mockLabsEnabled,
     mockLabsDisabled,
     mockWebhookRequests,

ghost/core/test/utils/e2e-framework.js

@@ -82,6 +82,9 @@ const startGhost = async (options = {}) => {
         await urlServiceUtils.isFinished();
     }
 
+    // Disable network in tests at the start
+    mockManager.disableNetwork();
+
     return ghostServer;
 };
 

ghost/core/test/utils/overrides.js

@@ -5,3 +5,15 @@ require('../../core/server/overrides');
 
 const {mochaHooks} = require('@tryghost/express-test').snapshot;
 exports.mochaHooks = mochaHooks;
+
+const mockManager = require('./e2e-framework-mock-manager');
+
+const originalBeforeAll = mochaHooks.beforeAll;
+mochaHooks.beforeAll = async function () {
+    if (originalBeforeAll) {
+        await originalBeforeAll();
+    }
+
+    // Disable network in tests to prevent any accidental requests
+    mockManager.disableNetwork();
+};