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

reference issue: assert whole error messages #13454

Closed
1 task done
refack opened this issue Jun 4, 2017 · 3 comments
Closed
1 task done

reference issue: assert whole error messages #13454

refack opened this issue Jun 4, 2017 · 3 comments
Labels
assert Issues and PRs related to the assert subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. test Issues and PRs related to the tests.

Comments

@refack
Copy link
Contributor

refack commented Jun 4, 2017

  • Version: *
  • Platform: *
  • Subsystem: test

It's a continuing goal that our test suite will assert full error message content (since it has been decided that changes in error messages is semver-major)
@refack refack added the test Issues and PRs related to the tests. label Jun 4, 2017
@refack
Copy link
Contributor Author

refack commented Jun 4, 2017

@nodejs/testing is this a dup? I couldn't find another.

@tniessen tniessen changed the title referance issue: assert whole error messages reference issue: assert whole error messages Jun 4, 2017
@vsemozhetbyt vsemozhetbyt added the errors Issues and PRs related to JavaScript errors originated in Node.js core. label Jun 4, 2017
@refack
Copy link
Contributor Author

refack commented Jun 4, 2017

@tniessen 10x!

@aniketshukla aniketshukla mentioned this issue Jun 4, 2017
Closed
3 tasks
@addaleax addaleax added the assert Issues and PRs related to the assert subsystem. label Jun 4, 2017
refack pushed a commit to refack/node that referenced this issue Jun 7, 2017
@aniketshukla @refack
test: validate full error messages
12e39d6 
test: changed test2 of test-vm-timeout.js so that entire error message
      would be matched in assert.throw.

      Before test 2 of test-vm-timeout.js would match any RangeError,
      now it looks specifically for the error message
      "RangeError: timeout must be a positive number"

test: changed test 3 of test-vm-timeout.js so that entire error message
      would be matched in assert.throw.

      Before test 3 of test-vm-timeout.js would match any RangeError,
      now it looks specifically for the error message
      "RangeError: timeout must be a positive number"

PR-URL: nodejs#13453
Refs: nodejs#13454
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
jasnell pushed a commit that referenced this issue Jun 7, 2017
@aniketshukla @jasnell
test: validate full error messages
757ae52 
test: changed test2 of test-vm-timeout.js so that entire error message
      would be matched in assert.throw.

      Before test 2 of test-vm-timeout.js would match any RangeError,
      now it looks specifically for the error message
      "RangeError: timeout must be a positive number"

test: changed test 3 of test-vm-timeout.js so that entire error message
      would be matched in assert.throw.

      Before test 3 of test-vm-timeout.js would match any RangeError,
      now it looks specifically for the error message
      "RangeError: timeout must be a positive number"

PR-URL: #13453
Refs: #13454
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
MylesBorins pushed a commit that referenced this issue Jul 17, 2017
@aniketshukla @MylesBorins
test: validate full error messages
779402e 
test: changed test2 of test-vm-timeout.js so that entire error message
      would be matched in assert.throw.

      Before test 2 of test-vm-timeout.js would match any RangeError,
      now it looks specifically for the error message
      "RangeError: timeout must be a positive number"

test: changed test 3 of test-vm-timeout.js so that entire error message
      would be matched in assert.throw.

      Before test 3 of test-vm-timeout.js would match any RangeError,
      now it looks specifically for the error message
      "RangeError: timeout must be a positive number"

PR-URL: #13453
Refs: #13454
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
@BridgeAR
Copy link
Member

Closing as outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
assert Issues and PRs related to the assert subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@refack @addaleax @BridgeAR @vsemozhetbyt