★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/26947Nouvelle recherche | Portail wanayoo
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

test: cover napi_get/set/has_named_property() #26947

Conversation

gabrielschulhof
Copy link
Contributor

@gabrielschulhof gabrielschulhof commented Mar 27, 2019

Add test coverage for these N-APIs.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

Add test coverage for these N-APIs.

PR-URL: nodejs#26947
@gabrielschulhof gabrielschulhof force-pushed the napi-get-set-has-named-property-test branch from e4005e5 to 48b9bfc Compare Apr 1, 2019
danbev
danbev approved these changes Apr 1, 2019

NAPI_ASSERT(env, argc >= 2, "Wrong number of arguments");

napi_valuetype valuetype0;
Copy link
Member

@danbev danbev Apr 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Perhaps it would be more consistent to name the NAPI variables like value_type0 instead of valuetype0, like is done for other variables like key_length above. Or is this intentionally done for the NAPI types?

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Apr 1, 2019

Copy link
Member

@mhdawson mhdawson left a comment

LGTM

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Apr 4, 2019

@gabrielschulhof
Copy link
Contributor Author

@gabrielschulhof gabrielschulhof commented Apr 5, 2019

Landed in baa54a5.

gabrielschulhof pushed a commit that referenced this issue Apr 5, 2019
Add test coverage for these N-APIs.

PR-URL: #26947
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@gabrielschulhof gabrielschulhof deleted the napi-get-set-has-named-property-test branch Apr 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants