-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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
test: add coverage for error apis #12729
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,3 +55,37 @@ assert.strictEqual(test_error.checkError({}), false, | |
// Test that non-error primitive is correctly classed | ||
assert.strictEqual(test_error.checkError('non-object'), false, | ||
'Non-error primitive correctly classed by napi_is_error'); | ||
|
||
assert.throws(() => { | ||
test_error.throwExistingError(); | ||
}, /Error: existing error/); | ||
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. We've been moving toward adding |
||
|
||
assert.throws(() => { | ||
test_error.throwError(); | ||
}, /Error: error/); | ||
|
||
assert.throws(() => { | ||
test_error.throwRangeError(); | ||
}, /RangeError: range error/); | ||
|
||
assert.throws(() => { | ||
test_error.throwTypeError(); | ||
}, /TypeError: type error/); | ||
|
||
let error = test_error.createError(); | ||
assert.ok(error instanceof Error, 'expected error to be an instance of Error'); | ||
assert.strictEqual(error.message, 'error', 'expected message to be "error"'); | ||
|
||
error = test_error.createRangeError(); | ||
assert.ok(error instanceof RangeError, | ||
'expected error to be an instance of RangeError'); | ||
assert.strictEqual(error.message, | ||
'range error', | ||
'expected message to be "range error"'); | ||
|
||
error = test_error.createTypeError(); | ||
assert.ok(error instanceof TypeError, | ||
'expected error to be an instance of TypeError'); | ||
assert.strictEqual(error.message, | ||
'type error', | ||
'expected message to be "type error"'); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,9 +15,64 @@ napi_value checkError(napi_env env, napi_callback_info info) { | |
return result; | ||
} | ||
|
||
napi_value throwExistingError(napi_env env, napi_callback_info info) { | ||
napi_value message; | ||
napi_value error; | ||
NAPI_CALL(env, napi_create_string_utf8(env, "existing error", -1, &message)); | ||
NAPI_CALL(env, napi_create_error(env, message, &error)); | ||
NAPI_CALL(env, napi_throw(env, error)); | ||
return nullptr; | ||
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. Just noting for the future: it would be excellent if these n-api thrown errors followed the same conventions as the 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. The 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. Opened nodejs/abi-stable-node#241 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. |
||
} | ||
|
||
napi_value throwError(napi_env env, napi_callback_info info) { | ||
NAPI_CALL(env, napi_throw_error(env, "error")); | ||
return nullptr; | ||
} | ||
|
||
napi_value throwRangeError(napi_env env, napi_callback_info info) { | ||
NAPI_CALL(env, napi_throw_range_error(env, "range error")); | ||
return nullptr; | ||
} | ||
|
||
napi_value throwTypeError(napi_env env, napi_callback_info info) { | ||
NAPI_CALL(env, napi_throw_type_error(env, "type error")); | ||
return nullptr; | ||
} | ||
|
||
napi_value createError(napi_env env, napi_callback_info info) { | ||
napi_value result; | ||
napi_value message; | ||
NAPI_CALL(env, napi_create_string_utf8(env, "error", -1, &message)); | ||
NAPI_CALL(env, napi_create_error(env, message, &result)); | ||
return result; | ||
} | ||
|
||
napi_value createRangeError(napi_env env, napi_callback_info info) { | ||
napi_value result; | ||
napi_value message; | ||
NAPI_CALL(env, napi_create_string_utf8(env, "range error", -1, &message)); | ||
NAPI_CALL(env, napi_create_range_error(env, message, &result)); | ||
return result; | ||
} | ||
|
||
napi_value createTypeError(napi_env env, napi_callback_info info) { | ||
napi_value result; | ||
napi_value message; | ||
NAPI_CALL(env, napi_create_string_utf8(env, "type error", -1, &message)); | ||
NAPI_CALL(env, napi_create_type_error(env, message, &result)); | ||
return result; | ||
} | ||
|
||
void Init(napi_env env, napi_value exports, napi_value module, void* priv) { | ||
napi_property_descriptor descriptors[] = { | ||
DECLARE_NAPI_PROPERTY("checkError", checkError), | ||
DECLARE_NAPI_PROPERTY("throwExistingError", throwExistingError), | ||
DECLARE_NAPI_PROPERTY("throwError", throwError), | ||
DECLARE_NAPI_PROPERTY("throwRangeError", throwRangeError), | ||
DECLARE_NAPI_PROPERTY("throwTypeError", throwTypeError), | ||
DECLARE_NAPI_PROPERTY("createError", createError), | ||
DECLARE_NAPI_PROPERTY("createRangeError", createRangeError), | ||
DECLARE_NAPI_PROPERTY("createTypeError", createTypeError), | ||
}; | ||
|
||
NAPI_CALL_RETURN_VOID(env, napi_define_properties( | ||
|
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.
If
throwExistingError()
and similar calls below don't actually throw an error, then this test would incorrectly pass, right?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.
Right. I'd suggest using
assert.throws()
to verify expected exceptions. It will make cleaner code anyway.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.
Agreed.
assert.throws()
is a much better choice here.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.
Agreed, changing.