-
-
Notifications
You must be signed in to change notification settings - Fork 32.7k
docs: stronger suggestion for userland assert #4535
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,9 +3,10 @@ | |
Stability: 3 - Locked | ||
|
||
The `assert` module provides a simple set of assertion tests that can be used | ||
to test invariants and implement unit tests. While the `assert` module is | ||
generally intended for internal use by Node.js itself, it can be used by user | ||
code calling `require('assert')`. | ||
to test invariants and implement unit tests. It is recommended that users of | ||
Node.js choose a third party assertion library from [npm][] instead of relying | ||
on `assert`. That being said, `assert` can be used by user code calling | ||
`require('assert')`. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor nit... I'd like to avoid starting sentences with a lower-cased module name, especially two sentences in a row... Perhaps reword the las two sentences to:
|
||
The API for the `assert` module is [Locked][]. This means that there will be no | ||
additions or changes to any of the methods implemented and exposed by | ||
|
@@ -357,6 +358,7 @@ Custom error validation: | |
); | ||
|
||
[Locked]: documentation.html#documentation_stability_index | ||
[npm]: https://www.npmjs.com/search?q=assertion+library | ||
[`assert.deepEqual`]: #assert_assert_deepequal_actual_expected_message | ||
[`assert.deepStrictEqual`]: #assert_assert_deepstrictequal_actual_expected_message | ||
[`assert.throws()`]: #assert_assert_throws_block_error_message | ||
|
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mikeal This statement should not be this strong. There is no commonly npm package in existence for asserting _invariants_ in source. At least, not that I know of, can you point to one?
I suggest we make clear that assert is NOT for unit tests, and not point people to non-existent modules for the use-case that assert is a good tool for (invariants).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I see, searching for assert on npmjs.org gets you pages of results, none of which are useful to replace assert for invariants, and none of which are a unit test framework (which is probably what the reader wanted).
The usage of
assert
we want to warn people away from is using it as a unit testing library, such as mocha, tap, and nunit, and none of those are even in the first page of search results when using npmjs.org search.Of course, I can't find a search term (other than "mocha") that yields mocha in the first page. :-( This is likely because npmjs.org search is unranked, so not so helpful, googling for node test framework works better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sam-github I'm a bit confused by your comments and feel that I must be misunderstanding something, so perhaps you could help me understand by rephrasing?
Isn't
assert
, for example, pretty much the same thing? That's the first result I get when I searchassert
.FWIW, there's also
invariant
which is dead simple (basically just likeassert()
).Do people somehow confuse the
assert
module as being a testing framework or library? Or do you mean it shouldn't be used for assertions in userland tests period? I think it's fairly commonly used for assertions within a framework like Mocha.