Added regression test pinning IndexNow ping URL output#27711
Merged
Conversation
Contributor
WalkthroughThis pull request adds a test case to the IndexNow service test suite that validates the Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This was referenced May 6, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ghost/core/test/unit/server/services/indexnow.test.js (1)
419-420: ⚡ Quick winAvoid coupling this regression to one internal URL API.
This suite is framed as output pinning, but stubbing only
getUrlByResourceIdmakes it fail on call-shape refactors even when?url=output remains correct. Stub both URL entry points so the test stays behavior-focused.Suggested adjustment
beforeEach(function () { sinon.stub(urlService, 'getUrlByResourceId').returns(POST_URL); + sinon.stub(urlService.facade, 'getUrlForResource').returns(POST_URL); requestStub = sinon.stub().resolves({statusCode: 200}); resetIndexNow = indexnow.__set__('request', requestStub);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ghost/core/test/unit/server/services/indexnow.test.js` around lines 419 - 420, The test currently stubs only urlService.getUrlByResourceId which couples the spec to that internal API; update the test to stub both public URL entry points (urlService.getUrlByResourceId and urlService.getUrlByResource) to return POST_URL so the assertion relies on the external ?url= behavior rather than a single internal method shape change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@ghost/core/test/unit/server/services/indexnow.test.js`:
- Around line 419-420: The test currently stubs only
urlService.getUrlByResourceId which couples the spec to that internal API;
update the test to stub both public URL entry points
(urlService.getUrlByResourceId and urlService.getUrlByResource) to return
POST_URL so the assertion relies on the external ?url= behavior rather than a
single internal method shape change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3fc08892-e36a-4a11-a561-d83565dab991
📒 Files selected for processing (28)
ghost/core/core/boot.jsghost/core/core/bridge.jsghost/core/core/frontend/helpers/authors.jsghost/core/core/frontend/helpers/tags.jsghost/core/core/frontend/meta/author-url.jsghost/core/core/frontend/meta/url.jsghost/core/core/frontend/services/routing/controllers/collection.jsghost/core/core/frontend/services/routing/controllers/email-post.jsghost/core/core/frontend/services/routing/controllers/previews.jsghost/core/core/frontend/services/routing/router-manager.jsghost/core/core/frontend/services/rss/generate-feed.jsghost/core/core/server/api/endpoints/utils/serializers/output/utils/url.jsghost/core/core/server/services/audience-feedback/audience-feedback-service.jsghost/core/core/server/services/comments/comments-service-emails.jsghost/core/core/server/services/email-service/email-renderer.jsghost/core/core/server/services/url/index.jsghost/core/core/server/services/url/url-service-facade.tsghost/core/test/unit/api/canary/utils/serializers/output/utils/url.test.jsghost/core/test/unit/frontend/helpers/authors.test.jsghost/core/test/unit/frontend/helpers/tags.test.jsghost/core/test/unit/frontend/meta/author-url.test.jsghost/core/test/unit/frontend/meta/url.test.jsghost/core/test/unit/frontend/services/routing/controllers/collection.test.jsghost/core/test/unit/frontend/services/rss/generate-feed.test.jsghost/core/test/unit/server/services/audience-feedback/audience-feedback-service.test.jsghost/core/test/unit/server/services/comments/comments-service-emails.test.jsghost/core/test/unit/server/services/indexnow.test.jsghost/core/test/unit/server/services/url/url-service-facade.test.js
allouis
force-pushed
the
hkg-1738-test-indexnow-url
branch
from
2162eca to
e213b15
Compare
2 tasks
allouis
force-pushed
the
hkg-1738-test-comments-url
branch
from
48cff0d to
83b184a
Compare
allouis
force-pushed
the
hkg-1738-test-indexnow-url
branch
2 times, most recently
from
7f43104 to
cdde506
Compare
allouis
force-pushed
the
hkg-1738-test-comments-url
branch
from
83b184a to
95acc5e
Compare
ref https://linear.app/ghost/issue/HKG-1767/ The IndexNow listener pings a URL derived from the URL service whenever a post's SEO-relevant fields change. The existing test stubs the URL service as a sanity check but never asserts on the URL that the request body actually carries, so a regression in how the listener resolves the post's URL would slip through. Driving indexnowListener end-to-end and pinning the ping URL through the public boundary closes that gap.
allouis
force-pushed
the
hkg-1738-test-indexnow-url
branch
from
cdde506 to
aaa0d82
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ghost/core/test/unit/server/services/indexnow.test.js`:
- Around line 419-438: The test currently stubs urlService.getUrlByResourceId
unconditionally so it returns POST_URL for any arg; change the setup and
assertion so the test verifies the lookup call shape: when calling ping(post)
ensure urlService.getUrlByResourceId is stubbed for the expected id (e.g., use
sinon.stub(urlService, 'getUrlByResourceId').withArgs(post.id).returns(POST_URL)
or alternatively keep the stub but assert the call) and add an assertion like
sinon.assert.calledOnceWith(urlService.getUrlByResourceId, post.id) after await
ping(post) to guarantee the function was invoked with post.id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b1651e2c-297d-4ffb-9a52-30a636f314a1
📒 Files selected for processing (1)
ghost/core/test/unit/server/services/indexnow.test.js
Comment on lines
+419
to
+438
| sinon.stub(urlService, 'getUrlByResourceId').returns(POST_URL); | ||
|
|
||
| requestStub = sinon.stub().resolves({statusCode: 200}); | ||
| resetIndexNow = indexnow.__set__('request', requestStub); | ||
|
|
||
| settingsCacheStub.withArgs('indexnow_api_key').returns('a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4'); | ||
| }); | ||
|
|
||
| afterEach(function () { | ||
| resetIndexNow(); | ||
| }); | ||
|
|
||
| it('passes the post URL into the IndexNow request', async function () { | ||
| const post = {id: 'abc', slug: 'some-post', type: 'post'}; | ||
|
|
||
| await ping(post); | ||
|
|
||
| sinon.assert.calledOnce(requestStub); | ||
| const indexNowUrl = new URL(requestStub.firstCall.args[0]); | ||
| assert.equal(indexNowUrl.searchParams.get('url'), POST_URL); |
Contributor
There was a problem hiding this comment.
The getUrlByResourceId stub is unconditional — the test's core regression scenario isn't actually pinned.
The PR's stated goal is to catch a future change that swaps getUrlByResourceId(post.id) for a different call shape (e.g., getUrlForResource(post)). However, the stub at line 419 has no withArgs constraint, so it returns POST_URL regardless of what argument is passed. The final assertion only checks that the URL value in the request matches POST_URL — it does not verify how that URL was looked up.
If ping() were accidentally changed to call getUrlByResourceId(post.url) or getUrlByResourceId(undefined), the stub would still resolve to POST_URL and the test would still pass, silently missing exactly the regression it was designed to catch.
Adding calledOnceWith(post.id) on the urlService.getUrlByResourceId spy is the minimal fix:
🔧 Proposed fix
sinon.assert.calledOnce(requestStub);
+ sinon.assert.calledOnceWith(urlService.getUrlByResourceId, 'abc');
const indexNowUrl = new URL(requestStub.firstCall.args[0]);
assert.equal(indexNowUrl.searchParams.get('url'), POST_URL);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ghost/core/test/unit/server/services/indexnow.test.js` around lines 419 -
438, The test currently stubs urlService.getUrlByResourceId unconditionally so
it returns POST_URL for any arg; change the setup and assertion so the test
verifies the lookup call shape: when calling ping(post) ensure
urlService.getUrlByResourceId is stubbed for the expected id (e.g., use
sinon.stub(urlService, 'getUrlByResourceId').withArgs(post.id).returns(POST_URL)
or alternatively keep the stub but assert the call) and add an assertion like
sinon.assert.calledOnceWith(urlService.getUrlByResourceId, post.id) after await
ping(post) to guarantee the function was invoked with post.id.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a unit test pinning the IndexNow ping URL output. Stubs
request(), drivesindexnowListeneragainst a fake post with an SEO-relevant change, and asserts the ping URL exactly matches the URL service's return value for that post.Why
We're about to migrate IndexNow's URL-service lookup from
getUrlByResourceId(post.id)tofacade.getUrlForResource(post). The existing test only stubs the URL service as a sanity check and never asserts on the URL the request body actually carries. Before the migration, we want a regression test pinning the ping URL through the public listener.This test lands on top of
mainand is independently mergeable — no production change.Stacked on #27710 because the broader refactor that follows modifies test files introduced by both PRs. Once #27710 merges, this PR's diff stays at 1 commit and its base auto-updates to
main.ref https://linear.app/ghost/issue/HKG-1767/
Test plan