test: checks on napi factory wrap’s finalization#22612
test: checks on napi factory wrap’s finalization#22612legendecas wants to merge 1 commit intonodejs:masterfrom
Conversation
|
@nodejs/n-api PTAL |
gabrielschulhof
left a comment
There was a problem hiding this comment.
I'm actually not sure that we need this PR given that we already test for this in https://github.com/nodejs/node/blob/master/test/addons-napi/8_passing_wrapped/test.js.
|
Nevertheless, I don't have any strong objections to landing this. |
|
@legendecas I suppose for completeness you could instead, in https://github.com/nodejs/node/blob/master/test/addons-napi/8_passing_wrapped/test.js, change the storage of obj2 = null;
global.gc();
assert.strictEqual(addon.finalizeCount(), 2);That would round out that test nicely. |
|
@gabrielschulhof thanks for reviewing😁. amended. It's true that finalizer has been checked in 8_passing_wrapped, however it could be more reasonable to take more checks. |
|
Resumed CI as: https://ci.nodejs.org/job/node-test-pull-request/17016/ |
|
Resumed again as CI: https://ci.nodejs.org/job/node-test-pull-request/17017/ |
|
Another new CI: https://ci.nodejs.org/job/node-test-pull-request/17145/ |
|
Resumed CI as: https://ci.nodejs.org/job/node-test-pull-request/17152/ |
|
Landed in 1cee085. |
Expect closing #22396
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes