node-addon-api icon indicating copy to clipboard operation
node-addon-api copied to clipboard

test: Add test coverage to TSFN::New() overloads

Open JckXia opened this issue 2 years ago • 2 comments

Adding test coverage for TSFN::New overloads where we provide the async resource name and/or the async resource object but no finalizer callbacks.

JckXia avatar Aug 27 '22 03:08 JckXia

Hi @JckXia ,

I took a look at this and have a few questions:

  1. This tests TypedThreadSafeFunction, do we have equivalent coverage for the non-typed ThreadSafeFunction...?
  2. I looked at all the TypedTSFN::New tests (including this PR) and I don't see these three... Maybe I overlooked them...?
    1. Callback [missing] Resource [missing] Finalizer [passed]
    2. Callback [missing] Resource [passed] Finalizer [passed]
    3. Callback [passed] Resource [missing] Finalizer [missing]

KevinEady avatar Sep 30 '22 14:09 KevinEady

Hey @KevinEady,

For your first question, not at the moment. My plan was to complete coverage for TypedThreadSafeFunction and port some of these tests over late.

For your second question, I think I may have missed them and have also made a mistake in the test coverage doc for TSFN::New. Will update accordingly.

Edit: I think [Callback [missing] Resource [passed] Finalizer [passed]] is covered here https://github.com/nodejs/node-addon-api/blob/main/test/typed_threadsafe_function/typed_threadsafe_function_ctx.cc#L49.

JckXia avatar Sep 30 '22 15:09 JckXia