-
Notifications
You must be signed in to change notification settings - Fork 459
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
Add test case for Object Set using uint32 as key #1130
Conversation
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.
LGTM!
My mistake, just modified. |
@meixg The change looks good! Thanks for working on this one. We currently have some issues with our CI but that should hopefully clear up soon. |
@meixg We've made a change to our windows CI that should unblock this PR. Please pull in the latest changes and push again. Thanks! |
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.
LGTM
@JckXia CI passed. |
Landed as |
Thanks for the contribution. And if you are interested in joining our weekly meetings on Friday at 11ET please feel free to do so. |
This PR covered
template void Set(uint32_t index, const ValueType& value)
Other testings for Object (#936) may have already been covered:
https://github.com/nodejs/node-addon-api/blob/main/test/object/object.cc#L234:
PropertyLValuestd::string operator [](const char* utf8name)
PropertyLValuestd::string operator [](const std::string& utf8name)
PropertyLValue<uint32_t> operator [](uint32_t index)
Object::PropertyLValue (Uncovered)
https://github.com/nodejs/node-addon-api/blob/main/test/object/has_property.cc#L26:
bool Has(uint32_t index) const
https://github.com/nodejs/node-addon-api/blob/main/test/object/get_property.cc#L18:
Value Get(uint32_t index) const
https://github.com/nodejs/node-addon-api/blob/main/test/object/delete_property.cc#L6:
bool Delete(uint32_t index)
https://github.com/nodejs/node-addon-api/blob/main/test/object/object.cc#L280:
bool InstanceOf(const Function& constructor) const