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.

Sign up for GitHub

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

Jump to bottom

Re-instantiate AssertionError under Node v8 (and v7) #26

Merged
merged 4 commits into from
Jun 9, 2017
Merged

Re-instantiate AssertionError under Node v8 (and v7) #26

merged 4 commits into from
Jun 9, 2017

Conversation

twada
Copy link
Member

@twada twada commented Jun 8, 2017 •
edited
Loading

Since their err.stack is unchanged even if err.message is updated.

  • Mocha uses err.message for reporting
  • But vanilla Node uses err.stack for reporting

Affects not only Node8 but Node7, so we cannot use err.code for feature detection (err.code is introduced in Node8).

Therefore, I've wrote a dirty feature detection code.
We should re-instantiate and throw AssertionError under Node8 (and Node7).

refs: power-assert-js/power-assert#85

twada added 3 commits June 8, 2017 23:58
@twada
fix: recreate AssertionError under Node v8,v7 and if message is modified
f34b6e7 
when `modifyMessageOnRethrow` option is truthy.
since Node v8 (and v7) does not update message on rethrow.

refs power-assert-js/power-assert#85
@twada
refactor: refine regex for more accurate feature detection
5004709
@twada
refactor: recreate AssertionError if necessary
f0aad9e 
recreate AssertionError when config.modifyMessageOnRethrow is false but config.saveContextOnRethrow is true
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.08%) to 97.222% when pulling f0aad9e on recreate-assertion-error into b833738 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.08%) to 97.222% when pulling f0aad9e on recreate-assertion-error into b833738 on master.

@coveralls
Copy link

coveralls commented Jun 9, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.3%) to 98.611% when pulling 3b25928 on recreate-assertion-error into b833738 on master.

@twada twada changed the title Recreate AssertionError under Node v8 (and v7) Re-instantiate AssertionError under Node v8 (and v7) Jun 9, 2017
@twada twada merged commit b75f3db into master Jun 9, 2017
@twada twada deleted the recreate-assertion-error branch June 9, 2017 14:41
twada added a commit to power-assert-js/power-assert that referenced this pull request Jun 10, 2017
@twada
fix: update empower to 1.2.3 for vanilla Node8 support
821cc02 
refs power-assert-js/empower#26
@twada twada mentioned this pull request Jun 10, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@twada @coveralls