🐛 Fixed flaky unit tests caused by DNS stubs lost after sinon.restore()

ghost/core/test/unit/server/lib/request-external.test.js

@@ -371,12 +371,6 @@ describe('External Request', function () {
     });
 
     describe('general behavior', function () {
-        beforeEach(function () {
-            sinon.stub(dnsPromises, 'lookup').callsFake(function () {
-                return Promise.resolve({address: '123.123.123.123'});
-            });
-        });
-
         afterEach(async function () {
             await configUtils.restore();
             sinon.restore();

ghost/core/test/unit/server/services/mail/ghost-mailer.test.js

@@ -1,4 +1,3 @@
-const dns = require('dns');
 const sinon = require('sinon');
 const mail = require('../../../../../core/server/services/mail');
 const settingsCache = require('../../../../../core/shared/settings-cache');
@@ -123,8 +122,6 @@ describe('Mail: Ghostmailer', function () {
             configUtils.set({mail: {}});
 
             mailer = new mail.GhostMailer();
-
-            sinon.stub(dns, 'resolveMx').yields(null, []);
         });
 
         afterEach(function () {

ghost/core/test/unit/server/services/mentions/mention-discovery-service.test.js

@@ -1,5 +1,5 @@
 const sinon = require('sinon');
-const dnsPromises = require('dns').promises;
+
 const assert = require('node:assert/strict');
 const nock = require('nock');
 
@@ -13,10 +13,6 @@ describe('MentionDiscoveryService', function () {
 
     beforeEach(function () {
         nock.disableNetConnect();
-        // externalRequest does dns lookup; stub to make sure we don't fail with fake domain names
-        sinon.stub(dnsPromises, 'lookup').callsFake(function () {
-            return Promise.resolve({address: '123.123.123.123'});
-        });
     });
 
     afterEach(function () {

ghost/core/test/unit/server/services/mentions/mention-sending-service.test.js

@@ -5,6 +5,7 @@ const nock = require('nock');
 const externalRequest = require('../../../../../core/server/lib/request-external.js');
 const sinon = require('sinon');
 const logging = require('@tryghost/logging');
+const dnsPromises = require('node:dns').promises;
 const {createModel} = require('./utils/index.js');
 
 // mock up job service
@@ -514,15 +515,22 @@ describe('MentionSendingService', function () {
         });
 
         it('Does not send to private IP behind DNS', async function () {
-            this.retries(1);
-            // Test that we don't make a request when a domain resolves to a private IP
-            // domaincontrol.com -> 127.0.0.1
-            const service = new MentionSendingService({externalRequest});
-            await assert.rejects(service.send({
-                source: new URL('https://example.com/source'),
-                target: new URL('https://target.com/target'),
-                endpoint: new URL('http://domaincontrol.com/webmentions')
-            }), /non-permitted private IP/);
+            // Stub DNS to return a private IP for the target domain
+            dnsPromises.lookup.restore?.();
+            const dnsStub = sinon.stub(dnsPromises, 'lookup').callsFake(() => {
+                return Promise.resolve({address: '127.0.0.1', family: 4});
+            });
+
+            try {
+                const service = new MentionSendingService({externalRequest});
+                await assert.rejects(service.send({
+                    source: new URL('https://example.com/source'),
+                    target: new URL('https://target.com/target'),
+                    endpoint: new URL('http://domaincontrol.com/webmentions')
+                }), /non-permitted private IP/);
+            } finally {
+                dnsStub.restore();
+            }
         });
     });
 });

ghost/core/test/utils/overrides.js

@@ -103,8 +103,19 @@ mochaHooks.afterEach = async function () {
 
     clearTimeout(timeout);
 
-    if (originalAfterEach) {
-        await originalAfterEach();
+    try {
+        if (originalAfterEach) {
+            await originalAfterEach();
+        }
+    } finally {
+        // Re-apply network disabling after each test. Individual test afterEach hooks
+        // often call sinon.restore() which removes the DNS stubs set up by
+        // disableNetwork() in beforeAll. Without re-applying, subsequent tests make
+        // real DNS lookups on nocked domains which can be slow on CI, causing flaky
+        // timeouts (e.g. ExternalMediaInliner CDN storage adapter tests).
+        // Wrapped in finally so DNS stubs are restored even if a previous afterEach
+        // hook throws.
+        mockManager.disableNetwork();
     }
 };