Skip to content
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

feature request for assert core module #5312

Closed
ORESoftware opened this issue Feb 18, 2016 · 9 comments
Closed

feature request for assert core module #5312

ORESoftware opened this issue Feb 18, 2016 · 9 comments
Labels
assert Issues and PRs related to the assert subsystem. feature request Issues that request new features to be added to Node.js.

Comments

@ORESoftware
Copy link
Contributor

ORESoftware commented Feb 18, 2016

I have a simple case

  .end(function(err, res) {
        assert(err == null,'Error is not null => '+ err.stack);

the problem is of course, if err is null, the assert message will still get evaluated, so it will throw a new/preemptive error saying

Uncaught TypeError: Cannot read property 'stack' of null

I know assert is stable and not likely to change, but the feature I am looking for is somthing like this

assert(err == null, function(){
  return 'Error is not null => '+ err.stack;
});

this of course means, that the callback will only be evaluated if the assertion fails

hope that makes sense, thanks!

@mscdex mscdex added assert Issues and PRs related to the assert subsystem. feature request Issues that request new features to be added to Node.js. labels Feb 19, 2016
@Trott
Copy link
Member

Trott commented Feb 19, 2016

Offhand, I'm unaware of any userland assertion testing library that has this feature, but it might be worth looking.

I suspect you already are doing something like this, but just in case, a simple workaround for your issue might look something like this:

const msg = err ? `Error is not null => ${err.stack}` : '';
assert(err == null, msg);

Or:

if (err) {
   assert.fail(null, null, `Error is not null => ${err.stack}`);
}

@ORESoftware
Copy link
Contributor Author

Yeah that will, I was kind of curious to see how useful this callback feature might be, probably not that useful and your suggestion should work, thanks

@thefourtheye
Copy link
Contributor

Node.js assertion module is not meant for general purpose use. Unless Node.js requires this feature, it wouldn't be implemented and as of now we don't have a pressing usecase.

@zeusdeux
Copy link
Contributor

@ORESoftware Since 81f2c9317aa9f9b4e69ed7a60dbecd8affd462a8 landed, you don't have to do err.stack anymore. By default, whenever an Error object is written out, it will print its stack too.

Do note that the commit hasn't landed in node 5.x and will not land until node 6.x.

@evanlucas
Copy link
Contributor

Closing as the assert module is locked. Thanks!

@mheiber
Copy link

mheiber commented Sep 2, 2016

@thefourtheye why is the assert module not meant for general purpose use? I've tried using it on small projects instead of things Shouldjs and have found it meets my needs well.

Basically the same question as this one on Stack Overflow

@Trott
Copy link
Member

Trott commented Sep 2, 2016

@mheiber #4532 and other issues allude to the reason the documentation recommends against using assert for unit testing: There are edge case bugs (or at least certainly surprises) and missing features.

A little more context: Knowing what we now know, if we were designing/building Node.js core all over again, the assert module would either not exist in Node.js or else consist of far fewer functions--quite possibly just assert() (which is currently an alias for assert.ok()).

The reasons for this, at least from my perspective are:

  • all the stuff being done in assert could easily be done in userland
  • core efforts are better spent elsewhere than perfecting a unit testing module that can be done in userland

There's additional context that others may choose to add here or not (such as why, all things being equal, we would favor keeping core small and doing things in userland). But that's the so-called 30,000 foot view.

Since assert has been in Node.js for a long time and a lot of the ecosystem depends on it, we are unlikely (at least as best as I can tell at the current time) to ever remove assert.throws() and friends. It would break too much stuff. But we can discourage people from using assert and encourage them to use userland modules that are maintained by people who care deeply about them and who aggressively fix edge-case bugs and who add cool new features when it makes sense. So that's what that's all about.

True, if you're doing straightforward assertions with simple cases, assert probably will meet your needs. But if you ever outgrow assert, you'll be better off with chai or whatever. So we encourage people to start there. It's better for them (usually) and better for us (usually).

I hope this is helpful and answers your question.

@mheiber
Copy link

mheiber commented Sep 2, 2016

@Trott this answer rules, thank you

@Trott
Copy link
Member

Trott commented Sep 2, 2016

@mheiber Heh, thanks. If you wish, consider upvoting the answer's twin at http://stackoverflow.com/a/39284536/436641 to increase its visibility there.

frou added a commit to frou/affa-guid that referenced this issue Feb 27, 2018
"Knowing what we now know, if we were designing/building Node.js core all over again, the assert module would either not exist in Node.js or else consist of far fewer functions--quite possibly just assert() (which is currently an alias for assert.ok())."

see nodejs/node#5312 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert Issues and PRs related to the assert subsystem. feature request Issues that request new features to be added to Node.js.
Projects
None yet
Development

No branches or pull requests

7 participants