Skip to content

Commit 07a4e87

Browse files
9larsons9larsons
authored
Changed IndexNow tests to avoid rewire (#28657)
no ref - exported IndexNow ping as a real service boundary - updated IndexNow unit tests to require the service normally instead of using rewire - exercised listener behavior through registered event handlers and HTTP behavior through nock
1 parent a5360a7 commit 07a4e87

2 files changed

Lines changed: 47 additions & 105 deletions

File tree

‎ghost/core/core/server/services/indexnow.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,5 +245,6 @@ function listen() {
245245

246246
module.exports = {
247247
listen: listen,
248+
ping: ping,
248249
getApiKey: getApiKey
249250
};

‎ghost/core/test/unit/server/services/indexnow.test.js‎

Lines changed: 46 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,8 @@ const assert = require('node:assert/strict');
22
const sinon = require('sinon');
33
const _ = require('lodash');
44
const nock = require('nock');
5-
const rewire = require('rewire');
6-
const errors = require('@tryghost/errors');
75
const testUtils = require('../../../utils');
8-
const indexnow = rewire('../../../../core/server/services/indexnow');
6+
const indexnow = require('../../../../core/server/services/indexnow');
97
const events = require('../../../../core/server/lib/common/events');
108
const settingsCache = require('../../../../core/shared/settings-cache');
119
const config = require('../../../../core/shared/config');
@@ -48,6 +46,18 @@ describe('IndexNow', function () {
4846
});
4947

5048
describe('listener()', function () {
49+
let listener;
50+
let urlStub;
51+
52+
beforeEach(function () {
53+
urlStub = sinon.stub(urlService.facade, 'getUrlForResource').returns('https://example.com/my-post/');
54+
settingsCacheStub.withArgs('indexnow_api_key').returns(null);
55+
loggingStub = sinon.stub(logging, 'warn');
56+
57+
indexnow.listen();
58+
listener = eventStub.firstCall.args[1];
59+
});
60+
5161
it('calls ping() with toJSONified model when content changed', function () {
5262
const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
5363

@@ -73,16 +83,10 @@ describe('IndexNow', function () {
7383
}
7484
};
7585

76-
const pingStub = sinon.stub().resolves();
77-
const resetIndexNow = indexnow.__set__('ping', pingStub);
78-
const listener = indexnow.__get__('indexnowListener');
79-
8086
listener(testModel);
8187

82-
sinon.assert.calledOnce(pingStub);
83-
sinon.assert.calledWith(pingStub, testPost);
84-
85-
resetIndexNow();
88+
sinon.assert.calledOnce(urlStub);
89+
sinon.assert.calledWith(urlStub, sinon.match({...testPost, type: 'posts'}), {absolute: true});
8690
});
8791

8892
it('does not call ping() when importing', function () {
@@ -100,15 +104,9 @@ describe('IndexNow', function () {
100104
}
101105
};
102106

103-
const pingStub = sinon.stub();
104-
const resetIndexNow = indexnow.__set__('ping', pingStub);
105-
const listener = indexnow.__get__('indexnowListener');
106-
107107
listener(testModel, {importing: true});
108108

109-
sinon.assert.notCalled(pingStub);
110-
111-
resetIndexNow();
109+
sinon.assert.notCalled(urlStub);
112110
});
113111

114112
it('does not call ping() when no SEO-relevant fields have changed', function () {
@@ -146,15 +144,9 @@ describe('IndexNow', function () {
146144
}
147145
};
148146

149-
const pingStub = sinon.stub();
150-
const resetIndexNow = indexnow.__set__('ping', pingStub);
151-
const listener = indexnow.__get__('indexnowListener');
152-
153147
listener(testModel);
154148

155-
sinon.assert.notCalled(pingStub);
156-
157-
resetIndexNow();
149+
sinon.assert.notCalled(urlStub);
158150
});
159151

160152
it('calls ping() when title changes', function () {
@@ -178,15 +170,9 @@ describe('IndexNow', function () {
178170
}
179171
};
180172

181-
const pingStub = sinon.stub().resolves();
182-
const resetIndexNow = indexnow.__set__('ping', pingStub);
183-
const listener = indexnow.__get__('indexnowListener');
184-
185173
listener(testModel);
186174

187-
sinon.assert.calledOnce(pingStub);
188-
189-
resetIndexNow();
175+
sinon.assert.calledOnce(urlStub);
190176
});
191177

192178
it('calls ping() when slug changes', function () {
@@ -210,15 +196,9 @@ describe('IndexNow', function () {
210196
}
211197
};
212198

213-
const pingStub = sinon.stub().resolves();
214-
const resetIndexNow = indexnow.__set__('ping', pingStub);
215-
const listener = indexnow.__get__('indexnowListener');
216-
217199
listener(testModel);
218200

219-
sinon.assert.calledOnce(pingStub);
220-
221-
resetIndexNow();
201+
sinon.assert.calledOnce(urlStub);
222202
});
223203

224204
it('calls ping() when meta_description changes', function () {
@@ -242,20 +222,13 @@ describe('IndexNow', function () {
242222
}
243223
};
244224

245-
const pingStub = sinon.stub().resolves();
246-
const resetIndexNow = indexnow.__set__('ping', pingStub);
247-
const listener = indexnow.__get__('indexnowListener');
248-
249225
listener(testModel);
250226

251-
sinon.assert.calledOnce(pingStub);
252-
253-
resetIndexNow();
227+
sinon.assert.calledOnce(urlStub);
254228
});
255229
});
256230

257231
describe('ping()', function () {
258-
const ping = indexnow.__get__('ping');
259232
let urlStub;
260233

261234
beforeEach(function () {
@@ -269,7 +242,7 @@ describe('IndexNow', function () {
269242
.reply(200);
270243
const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
271244

272-
await ping(testPost);
245+
await indexnow.ping(testPost);
273246

274247
sinon.assert.calledOnce(loggingStub);
275248
assert.equal(loggingStub.args[0][0].event.name, 'indexnow.pinged');
@@ -284,7 +257,7 @@ describe('IndexNow', function () {
284257
.reply(200);
285258
const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
286259

287-
await ping(testPost);
260+
await indexnow.ping(testPost);
288261

289262
assert.equal(pingRequest.isDone(), false);
290263
sinon.assert.calledOnce(loggingStub);
@@ -303,7 +276,7 @@ describe('IndexNow', function () {
303276

304277
testPost.slug = 'welcome';
305278

306-
await ping(testPost);
279+
await indexnow.ping(testPost);
307280

308281
assert.equal(pingRequest.isDone(), false);
309282
});
@@ -314,7 +287,7 @@ describe('IndexNow', function () {
314287
.reply(200);
315288
const testPage = _.clone(testUtils.DataGenerator.Content.posts[5]);
316289

317-
await ping(testPage);
290+
await indexnow.ping(testPage);
318291

319292
assert.equal(pingRequest.isDone(), false);
320293
});
@@ -327,7 +300,7 @@ describe('IndexNow', function () {
327300
.reply(200);
328301
const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
329302

330-
await ping(testPost);
303+
await indexnow.ping(testPost);
331304

332305
assert.equal(pingRequest.isDone(), false);
333306
});
@@ -340,7 +313,7 @@ describe('IndexNow', function () {
340313
.reply(200);
341314
const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
342315

343-
await ping(testPost);
316+
await indexnow.ping(testPost);
344317

345318
assert.equal(pingRequest.isDone(), false);
346319
});
@@ -353,7 +326,7 @@ describe('IndexNow', function () {
353326
.reply(200);
354327
const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
355328

356-
await ping(testPost);
329+
await indexnow.ping(testPost);
357330

358331
assert.equal(pingRequest.isDone(), false);
359332
});
@@ -367,7 +340,7 @@ describe('IndexNow', function () {
367340
.reply(200);
368341
const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
369342

370-
await ping(testPost);
343+
await indexnow.ping(testPost);
371344

372345
// Should NOT have made the ping request
373346
assert.equal(pingRequest.isDone(), false);
@@ -384,7 +357,7 @@ describe('IndexNow', function () {
384357
.reply(202);
385358
const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
386359

387-
await ping(testPost);
360+
await indexnow.ping(testPost);
388361

389362
assert.equal(pingRequest.isDone(), true);
390363
sinon.assert.calledOnce(loggingStub);
@@ -399,7 +372,7 @@ describe('IndexNow', function () {
399372
.reply(400);
400373
const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
401374

402-
await ping(testPost);
375+
await indexnow.ping(testPost);
403376

404377
assert.equal(pingRequest.isDone(), true);
405378
sinon.assert.calledOnce(loggingStub);
@@ -414,7 +387,7 @@ describe('IndexNow', function () {
414387
.reply(422);
415388
const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
416389

417-
await ping(testPost);
390+
await indexnow.ping(testPost);
418391

419392
assert.equal(pingRequest.isDone(), true);
420393
sinon.assert.calledOnce(loggingStub);
@@ -429,7 +402,7 @@ describe('IndexNow', function () {
429402
.reply(429);
430403
const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
431404

432-
await ping(testPost);
405+
await indexnow.ping(testPost);
433406

434407
assert.equal(pingRequest.isDone(), true);
435408
sinon.assert.calledOnce(loggingStub);
@@ -446,7 +419,7 @@ describe('IndexNow', function () {
446419
.reply(204);
447420
const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
448421

449-
await ping(testPost);
422+
await indexnow.ping(testPost);
450423

451424
assert.equal(pingRequest.isDone(), true);
452425
sinon.assert.calledOnce(loggingStub);
@@ -456,33 +429,17 @@ describe('IndexNow', function () {
456429
});
457430

458431
describe('ping() error classification (got HTTPError shape)', function () {
459-
const ping = indexnow.__get__('ping');
460-
let resetIndexNow;
461-
462432
beforeEach(function () {
463433
sinon.stub(urlService.facade, 'getUrlForResource').returns('https://example.com/my-post/');
464434
loggingStub = sinon.stub(logging, 'warn');
465435
});
466436

467-
afterEach(function () {
468-
if (resetIndexNow) {
469-
resetIndexNow();
470-
resetIndexNow = null;
471-
}
472-
});
473-
474-
function makeHttpError(statusCode) {
475-
const err = new Error(`Response code ${statusCode}`);
476-
err.name = 'HTTPError';
477-
err.code = 'ERR_NON_2XX_3XX_RESPONSE';
478-
err.response = {statusCode};
479-
return err;
480-
}
481-
482437
async function pingWithHttpError(statusCode) {
483-
resetIndexNow = indexnow.__set__('request', sinon.stub().rejects(makeHttpError(statusCode)));
438+
nock('https://api.indexnow.org')
439+
.get(/\/indexnow/)
440+
.reply(statusCode);
484441
const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
485-
await ping(testPost);
442+
await indexnow.ping(testPost);
486443
}
487444

488445
it('classifies a 429 (status on err.response.statusCode) as rate_limited', async function () {
@@ -516,18 +473,6 @@ describe('IndexNow', function () {
516473
assert.equal(loggingStub.args[0][0].event.name, 'indexnow.ping_failed');
517474
assert.equal(loggingStub.args[0][0].http.response.status_code, 503);
518475
});
519-
520-
it('still classifies a GhostError carrying err.statusCode (manual throw) correctly', async function () {
521-
const err = new errors.TooManyRequestsError({message: 'manual', statusCode: 429});
522-
resetIndexNow = indexnow.__set__('request', sinon.stub().rejects(err));
523-
const testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
524-
525-
await ping(testPost);
526-
527-
sinon.assert.calledOnce(loggingStub);
528-
assert.equal(loggingStub.args[0][0].event.name, 'indexnow.rate_limited');
529-
assert.equal(loggingStub.args[0][0].http.response.status_code, 429);
530-
});
531476
});
532477

533478
describe('getApiKey()', function () {
@@ -554,11 +499,9 @@ describe('IndexNow', function () {
554499
// for a resource-based facade method) could regress without anyone
555500
// noticing.
556501
describe('ping() URL output', function () {
557-
const ping = indexnow.__get__('ping');
558502
const POST_URL = 'https://my-blog.example/some-post/';
559503
let getUrlForResourceStub;
560-
let requestStub;
561-
let resetIndexNow;
504+
let pingRequest;
562505

563506
beforeEach(function () {
564507
// Bind the stub to the exact resource shape production passes
@@ -569,25 +512,23 @@ describe('IndexNow', function () {
569512
.withArgs(sinon.match({id: 'abc', type: 'posts'}), {absolute: true})
570513
.returns(POST_URL);
571514

572-
requestStub = sinon.stub().resolves({statusCode: 200});
573-
resetIndexNow = indexnow.__set__('request', requestStub);
515+
pingRequest = nock('https://api.indexnow.org')
516+
.get('/indexnow')
517+
.query((query) => {
518+
return query.url === POST_URL;
519+
})
520+
.reply(200);
574521

575522
settingsCacheStub.withArgs('indexnow_api_key').returns('a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4');
576523
});
577524

578-
afterEach(function () {
579-
resetIndexNow();
580-
});
581-
582525
it('passes the post URL into the IndexNow request', async function () {
583526
const post = {id: 'abc', slug: 'some-post', type: 'post'};
584527

585-
await ping(post);
528+
await indexnow.ping(post);
586529

587530
sinon.assert.calledOnce(getUrlForResourceStub);
588-
sinon.assert.calledOnce(requestStub);
589-
const indexNowUrl = new URL(requestStub.firstCall.args[0]);
590-
assert.equal(indexNowUrl.searchParams.get('url'), POST_URL);
531+
assert.equal(pingRequest.isDone(), true);
591532
});
592533
});
593534
});

0 commit comments

Comments
 (0)