Skip to content

Conversation

@gabrielschulhof
Copy link
Contributor

Add test coverage for passing NULL to each parameter of
napi.*(propert|element). In the case of napi_define_properties also
test setting various initializer fields to NULL.

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

@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Jan 24, 2020
@gabrielschulhofgabrielschulhof added the node-api Issues and PRs related to the Node-API. label Jan 24, 2020
@gabrielschulhofgabrielschulhofforce-pushed the test-null-for-object-properties branch 2 times, most recently from f7b7348 to f78645dCompareJanuary 24, 2020 02:59
Add test coverage for passing `NULL` to each parameter of `napi.*(propert|element)` and `napi_set_prototype`. In the case of `napi_define_properties` also test setting various initializer fields to `NULL`.
@gabrielschulhofgabrielschulhofforce-pushed the test-null-for-object-properties branch from f78645d to 5f048fdCompareJanuary 24, 2020 15:41
@gabrielschulhof
Copy link
ContributorAuthor

@devnexen done.

@nodejs-github-bot
Copy link
Collaborator

@TrottTrott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 26, 2020
@Trott
Copy link
Member

Landed in 42995a3

@TrottTrott closed this Jan 26, 2020
Trott pushed a commit that referenced this pull request Jan 26, 2020
Add test coverage for passing `NULL` to each parameter of `napi.*(propert|element)` and `napi_set_prototype`. In the case of `napi_define_properties` also test setting various initializer fields to `NULL`. PR-URL: #31488 Reviewed-By: David Carlier <[email protected]> Reviewed-By: Rich Trott <[email protected]>
codebytere pushed a commit that referenced this pull request Feb 17, 2020
Add test coverage for passing `NULL` to each parameter of `napi.*(propert|element)` and `napi_set_prototype`. In the case of `napi_define_properties` also test setting various initializer fields to `NULL`. PR-URL: #31488 Reviewed-By: David Carlier <[email protected]> Reviewed-By: Rich Trott <[email protected]>
@codebyterecodebytere mentioned this pull request Feb 17, 2020
@codebytere
Copy link
Member

@gabrielschulhof this seems to have caused a deterministic error in v12.x; is it intended to go back there? If yes, can you please open a manual backport? I've set the label but feel free to update it

Details
Error: Command failed: /Users/codebytere/Developer/node/out/Release/node /Users/codebytere/Developer/node/deps/npm/node_modules/node-gyp/bin/node-gyp.js rebuild --directory=/Users/codebytere/Developer/node/test/js-native-api/test_object ../test_null.c:321:23: warning: implicit declaration of function 'napi_get_all_property_names' is invalid in C99 [-Wimplicit-function-declaration] napi_get_all_property_names(NULL, ^ ../test_null.c:323:51: error: use of undeclared identifier 'napi_key_own_only' napi_key_own_only, ^ ../test_null.c:324:51: error: use of undeclared identifier 'napi_key_writable' napi_key_writable, ^ ../test_null.c:325:51: error: use of undeclared identifier 'napi_key_keep_numbers' napi_key_keep_numbers, ^ ../test_null.c:330:31: error: use of undeclared identifier 'napi_key_own_only' napi_key_own_only, ^ ../test_null.c:331:31: error: use of undeclared identifier 'napi_key_writable' napi_key_writable, ^ ../test_null.c:332:31: error: use of undeclared identifier 'napi_key_keep_numbers' napi_key_keep_numbers, ^ ../test_null.c:338:31: error: use of undeclared identifier 'napi_key_own_only' napi_key_own_only, ^ ../test_null.c:339:31: error: use of undeclared identifier 'napi_key_writable' napi_key_writable, ^ ../test_null.c:340:31: error: use of undeclared identifier 'napi_key_keep_numbers' napi_key_keep_numbers, ^ 1 warning and 9 errors generated. make[2]: *** [Release/obj.target/test_object/test_null.o] Error 1 at ChildProcess.exithandler (child_process.js:303:12) at ChildProcess.emit (events.js:311:20) at maybeClose (internal/child_process.js:1021:16) at Process.ChildProcess._handle.onexit (internal/child_process.js:286:5){killed: false, code: 1, signal: null, cmd: '/Users/codebytere/Developer/node/out/Release/node /Users/codebytere/Developer/node/deps/npm/node_modules/node-gyp/bin/node-gyp.js rebuild --directory=/Users/codebytere/Developer/node/test/js-native-api/test_object', stdout: ' CC(target) Release/obj.target/test_object/../common.o\n' + ' CC(target) Release/obj.target/test_object/../entry_point.o\n' + ' CC(target) Release/obj.target/test_object/test_null.o\n', stderr: "../test_null.c:321:23: warning: implicit declaration of function 'napi_get_all_property_names' is invalid in C99 [-Wimplicit-function-declaration]\n" + ' napi_get_all_property_names(NULL,\n' + ' ^\n' + "../test_null.c:323:51: error: use of undeclared identifier 'napi_key_own_only'\n" + ' napi_key_own_only,\n' + ' ^\n' + "../test_null.c:324:51: error: use of undeclared identifier 'napi_key_writable'\n" + ' napi_key_writable,\n' + ' ^\n' + "../test_null.c:325:51: error: use of undeclared identifier 'napi_key_keep_numbers'\n" + ' napi_key_keep_numbers,\n' + ' ^\n' + "../test_null.c:330:31: error: use of undeclared identifier 'napi_key_own_only'\n" + ' napi_key_own_only,\n' + ' ^\n' + "../test_null.c:331:31: error: use of undeclared identifier 'napi_key_writable'\n" + ' napi_key_writable,\n' + ' ^\n' + "../test_null.c:332:31: error: use of undeclared identifier 'napi_key_keep_numbers'\n" + ' napi_key_keep_numbers,\n' + ' ^\n' + "../test_null.c:338:31: error: use of undeclared identifier 'napi_key_own_only'\n" + ' napi_key_own_only,\n' + ' ^\n' + "../test_null.c:339:31: error: use of undeclared identifier 'napi_key_writable'\n" + ' napi_key_writable,\n' + ' ^\n' + "../test_null.c:340:31: error: use of undeclared identifier 'napi_key_keep_numbers'\n" + ' napi_key_keep_numbers,\n' + ' ^\n' + '1 warning and 9 errors generated.\n' + 'make[2]: *** [Release/obj.target/test_object/test_null.o] Error 1\n' } make[1]: *** [test/js-native-api/.buildstamp] Error 1 make[1]: *** Waiting for unfinished jobs.... 

gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Apr 2, 2020
Add test coverage for passing `NULL` to each parameter of `napi.*(propert|element)` and `napi_set_prototype`. In the case of `napi_define_properties` also test setting various initializer fields to `NULL`. PR-URL: nodejs#31488 Reviewed-By: David Carlier <[email protected]> Reviewed-By: Rich Trott <[email protected]>
@gabrielschulhof
Copy link
ContributorAuthor

@codebytere please see #32482 which backports the remaining bits of N-API 6 to v12.x.

@gabrielschulhofgabrielschulhof deleted the test-null-for-object-properties branch April 21, 2020 02:58
@targostargos added backport-open-v12.x and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-requested-v12.x labels Apr 25, 2020
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
Add test coverage for passing `NULL` to each parameter of `napi.*(propert|element)` and `napi_set_prototype`. In the case of `napi_define_properties` also test setting various initializer fields to `NULL`. Backport-PR-URL: nodejs#32482 PR-URL: nodejs#31488 Reviewed-By: David Carlier <[email protected]> Reviewed-By: Rich Trott <[email protected]>
targos pushed a commit that referenced this pull request Apr 28, 2020
Add test coverage for passing `NULL` to each parameter of `napi.*(propert|element)` and `napi_set_prototype`. In the case of `napi_define_properties` also test setting various initializer fields to `NULL`. Backport-PR-URL: #32482 PR-URL: #31488 Reviewed-By: David Carlier <[email protected]> Reviewed-By: Rich Trott <[email protected]>
@targostargos mentioned this pull request May 2, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

node-apiIssues and PRs related to the Node-API.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@gabrielschulhof@nodejs-github-bot@Trott@codebytere@devnexen@targos