Skip to content

Replaced value.should.be.empty() with node:assert#26428

Merged
EvanHahn merged 1 commit intomainfrom
should-be.empty-begone
Feb 16, 2026
Merged

Replaced value.should.be.empty() with node:assert#26428
EvanHahn merged 1 commit intomainfrom
should-be.empty-begone

Conversation

@EvanHahn
Copy link
Contributor

This test-only change should have no user impact.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 16, 2026

No actionable comments were generated in the recent review. 🎉


Walkthrough

This PR standardizes test assertions across multiple suites: it replaces Should.js assertions with Node's assert (including assert.deepEqual) and explicit type/truthiness checks, converts iterative membership checks to Set-based equality, replaces some lodash isPlainObject/empty checks with direct deep-equality checks, adds an assertExists import in one test, and modernizes sinon verifications to sinon.assert.calledOnceWithExactly(). No runtime code or exported/public signatures were changed; modifications are limited to test assertion styles and related test imports.

Possibly related PRs

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: replacing should.be.empty() assertions with node:assert equivalents across test files.
Description check ✅ Passed The description is directly related to the changeset, accurately noting this is a test-only change with no user impact that replaces should.be.empty() with node:assert.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch should-be.empty-begone

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.40.5)
ghost/core/test/legacy/models/model-posts.test.js

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.


In `@ghost/core/test/e2e-api/admin/integrations.test.js`:
- Around line 351-353: The inner callback parameter named "res" in the test
inside integrations.test.js shadows the outer "res" constant; rename the inner
parameter (for example to "response" or "result") and update its usage in the
arrow callback (e.g. assert.deepEqual(response.body, {})) to avoid the ESLint
no-shadow violation while preserving the exact assertion logic in the test
function.

@EvanHahn EvanHahn force-pushed the should-be.empty-begone branch from 94aa3c0 to 4fbb9f5 Compare February 16, 2026 21:42
@EvanHahn EvanHahn enabled auto-merge (squash) February 16, 2026 21:42
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.


In `@ghost/core/test/e2e-api/admin/redirects.test.js`:
- Line 2: Remove the unused import of the should assertion library: delete the
require statement that assigns to the symbol "should" (the top-level const
should = require('should');) since all assertions use Node's assert.deepEqual(),
leaving no references to "should" in the file; ensure no other references remain
after removal and run the tests to verify.
🧹 Nitpick comments (1)
🤖 Fix all nitpicks with AI agents
Verify each finding against the current code and only fix it if needed.


In `@ghost/core/test/e2e-api/admin/redirects.test.js`:
- Line 2: Remove the unused import of the should assertion library: delete the
require statement that assigns to the symbol "should" (the top-level const
should = require('should');) since all assertions use Node's assert.deepEqual(),
leaving no references to "should" in the file; ensure no other references remain
after removal and run the tests to verify.
ghost/core/test/e2e-api/admin/redirects.test.js (1)

2-2: Remove unused should import.

The should module is imported but no longer used in this file. All assertions use assert.deepEqual() from Node's strict assertion module.

Proposed fix
 const assert = require('node:assert/strict');
-const should = require('should');
 const supertest = require('supertest');
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ghost/core/test/e2e-api/admin/redirects.test.js` at line 2, Remove the unused
import of the should assertion library: delete the require statement that
assigns to the symbol "should" (the top-level const should = require('should');)
since all assertions use Node's assert.deepEqual(), leaving no references to
"should" in the file; ensure no other references remain after removal and run
the tests to verify.

This test-only change should have no user impact.
@EvanHahn EvanHahn force-pushed the should-be.empty-begone branch from 4fbb9f5 to a0a1e47 Compare February 16, 2026 22:11
@EvanHahn EvanHahn merged commit 323d70f into main Feb 16, 2026
33 checks passed
@EvanHahn EvanHahn deleted the should-be.empty-begone branch February 16, 2026 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant