docs: modernize spies example, drop should.js#6016
Open
gaurav0107 wants to merge 1 commit into
Open
Conversation
The spies explainer recommended should.js (deprecated 2020) and used broken syntax (`spy.called.should.equal.true` reads a property and discards the value rather than asserting). Switch the first example to sinon-only assertions and the "no library" example to node:assert, so both code blocks are copy-paste runnable today. refs mochajs#5445
|
|
This comment has been minimized.
This comment has been minimized.
mark-wiemer
added
the
status: needs review
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
2 participants
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.
PR Checklist
status: accepting prsOverview
The Spies explainer was flagged in #5445 as recommending should.js,
which has been deprecated since 2020. On a closer read the page also
contained syntactically broken examples:
spy.called.should.equal.true;is a no-op (reads a property anddrops the value); the correct should.js form would be
spy.called.should.be.trueorspy.called.should.equal(true).library yet relied on
.should.equal(...)from should.js.This PR:
(
sinon.assert.calledOnce,sinon.assert.calledWith); the pagealready uses these on the second test in the same block.
node:assert, matching therest of the docs site (e.g.
getting-started.mdxusesnode:assert).The broader audit of should.js mentions across other docs pages
remains tracked under #5445; this PR is intentionally scoped to the
spies explainer to keep the change focused.
No library code or workflow changes.
Note: #5445 is a meta-tracking issue per the maintainer's comment,
so this PR uses
fixesto satisfy the PR template format but doesnot aim to fully close the broader audit.