nodejs / js-native-api-test

Node.js js-native-api-test suite, externalized so that it can be used to test Node-API implementations outside of Node.js
MIT License
7 stars 10 forks source link

Concept and approach #7

Open mhdawson opened 3 years ago

mhdawson commented 3 years ago

We had a discussion today and started to think about how we could enable the test suite to be re-used.

The baseline is that we want to sync the contents of https://github.com/nodejs/node/tree/master/test/js-native-api regularly to the tests directory in this repo.
We then need to be able to build and run the test suite. What we were talking about around the end of the discussion (I've expanded on the concept) was something like this:

Existing tests also use the following so additional support may be needed or the tests would have to be updated

Building notes, we need to use the following for each test file:

mhdawson commented 3 years ago

The tests also use:

midawson@drx-hemera js-native-api]$ grep -r require * |grep -v build |grep -v assert |grep -v common
test_exception/test.js:// phase are propagated through require() into JavaScript.
test_exception/testFinalizerException.js:const { spawnSync } = require('child_process');
test_general/testInstanceOf.js:const fs = require('fs');
test_general/testInstanceOf.js:const path = require('path');
test_general/testEnvCleanup.js:  const { spawnSync } = require('child_process');
test_instance_data/test.js:if (module !== require.main) {
test_instance_data/test.js:  // When required as a module, run the tests.
test_instance_data/test.js:  checkOutput(requireAs(__filename, ['--expose-gc'], runOptions, 'child'));
test_instance_data/test.js:  checkOutput(requireAs(__filename, ['--expose-gc'], runOptions, 'worker'));
test_reference/test.js:// required to ensure delete could be called before

so those tests might need to be changed, excluded or the requirements on the implementation expanded.

mhdawson commented 3 years ago

If we were willing to limit to shared library instead of allowing for static linking we could extend so that the test runner is compiled against the headers, and accepted the path to a shared library to load/use at runtime. That would allow the build of the test runner to be self-contained.

vmoroz commented 3 years ago

What if we stop using JavaScript to drive our unit tests and create a test suite based on GTest? Existing Node.js tests are driven from JavaScript and many of them rely on Node-API to run. It typically looks like this:

There are a lot of JS <-> native interaction using Node-API before we even run a test. What if instead of writing tests in JavaScript, we write them in C++? This way we can test methods one by one, avoid the requirement for require, and make the testing suite to be more self-contained.

As an example, please have a look at the JSI unit tests. They are literally just two C++ files that can be used for different JSI implementations. JSI is the JavaScript Interface developed by Facebook. https://github.com/facebook/hermes/tree/master/API/jsi/jsi/test

These tests often run small JavaScript code snippets to observe results of the API calls. They first test that it can run code snippets. In Node-API it could be the napi_run_script function. Then, it wraps up the call into a C++ eval function and uses it for the tests.

The main advantage of this approach is that we do not need to simulate a big chunk of Node.js code. The disadvantage is that we have to convert existing tests.

I initially started to write such unit tests and was about 60% done, but abandoned this approach because I did not know how to synchronize them with the existing JavaScript-based tests. If we have these tests in this repo, then we would not have the code synchronization issues. You can see my work in progress here: https://github.com/vmoroz/v8-jsi/blob/6b180828d3aa55fd051d2b91deb871ba33666cf8/src/napi/test/napitest.cpp This is an unpolished code of a draft quality which was then removed, but I have it in the change history and it can be used as a starting point.