-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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 napi_property_descriptor #13510
Conversation
We did not have test coverage for using a napi_value pointing to a string or symbol for the name when creating a property. Add that coverage.
Should we also have a test case that uses a symbol as a property name? |
@jasongin added commit to add test for creating property with a symbol. |
Killed one part of one run on windows (after 4 hours) https://ci.nodejs.org/job/node-test-binary-windows/9109/RUN_SUBSET=0,VS_VERSION=vs2015,label=win2012r2/ |
I had accidentally kicked off 2 CI runs, the the link to the one posted in this PR passed, so issue is likely not related to PR. |
Failure looks infra related as it did not get to the point of running tests. |
Landed as 01f4d9a |
We did not have test coverage for using a napi_value pointing to a string or symbol for the name when creating a property. Add that coverage. PR-URL: #13510 Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
We did not have test coverage for using a napi_value pointing to a string or symbol for the name when creating a property. Add that coverage. PR-URL: #13510 Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
We did not have test coverage for using a napi_value pointing to a string or symbol for the name when creating a property. Add that coverage. PR-URL: nodejs#13510 Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
We did not have test coverage for using a napi_value
pointing to a string or symbol for the name when
creating a property. Add that coverage.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
test, n-api