enable unit tests #1078

deepakrkris · 2021-09-23T18:10:32Z

Enable running tests with specific filter conditions:

Example:

compile ad run only tests on objectwrap.cc and objectwrap.js

    npm run unit --filter=objectwrap

Wildcards are also possible:

Example:

compile and run all tests files ending with reference -> function_reference.cc object_reference.cc reference.cc

    npm run unit --filter=*reference

Multiple filter conditions are also allowed

Example:

compile and run all tests under folders threadsafe_function and typed_threadsafe_function and also the objectwrap.cc file

    npm run unit --filter='*function objectwrap'

deepakrkris · 2021-09-23T18:12:30Z

@mhdawson @KevinEady @gabrielschulhof PR is ready for review

gabrielschulhof · 2021-10-01T15:01:53Z

package.json

@@ -378,7 +378,9 @@
    "dev:incremental": "node test",
    "doc": "doxygen doc/Doxyfile",
    "lint": "eslint $(git diff --name-only refs/remotes/origin/main '**/*.js' | xargs) && node tools/clang-format",
-    "lint:fix": "node tools/clang-format --fix && eslint --fix $(git diff --cached --name-only '**/*.js' | xargs && git diff --name-only '**/*.js' | xargs)"
+    "lint:fix": "node tools/clang-format --fix && eslint --fix $(git diff --cached --name-only '**/*.js' | xargs && git diff --name-only '**/*.js' | xargs)",
+    "preunit": "filter=\"$npm_config_filter\" node-gyp rebuild -C unit-test",


Why do you need to copy $npm_config_filter into $filter? You can process $npm_config_filter directly in binding.gyp.

@gabrielschulhof , I wanted to refer to the passed in argument "filter" by it's original name, because npm translates it into npm_config_filter

OK. I understand. Still, I think for the simplicity of the command line it would be better to move this assignment into the JS file. So, like

// npm prepends "npm_config_" to all environment variables created from npm command line parameters. let filter = process.env.npm_config_filter;

You can drop "filter=\"$npm_config_filter\" because you're using process.env.npm_config_filter inside the script now.

gabrielschulhof · 2021-10-01T15:03:27Z

test/index.js

@@ -29,9 +29,23 @@ if (typeof global.gc !== 'function') {

 const fs = require('fs');
 const path = require('path');
+process.env.filter = require('../unit-test/matchModules').matchWildCards(process.env.filter);


This could easily be process.env.npm_config_filter.

@gabrielschulhof IMO using npm_config_filter seems slightly misleading, that is why I translated to "filter" , But I am open to changing this

gabrielschulhof · 2021-10-01T15:05:18Z

unit-test/.gitignore

@@ -0,0 +1,3 @@
+/node_modules
+/build
+/generated


Please make sure there are newlines at the ends of files!

@gabrielschulhof agreed

gabrielschulhof · 2021-10-01T15:07:21Z

unit-test/binding.gyp

+      'dependencies': [ 'generateBindingCC' ]
+    },
+  ],
+}


Please add a newline!

@gabrielschulhof thanks, agreed

gabrielschulhof · 2021-10-01T15:08:19Z

unit-test/binding.gyp

@@ -0,0 +1,64 @@
+{
+  'target_defaults': {
+    'includes': ['common.gypi'],


Why not ../common.gypi and then you don't need to copy the file?

@gabrielschulhof agreed

gabrielschulhof · 2021-10-01T15:14:18Z

unit-test/generate-binding-cc.js

+ *
+ * Test cases
+ * @fires only when run directly from terminal with TEST=true
+ *
+ *
+ *
+ * 


Do we need the blank lines surrounding the comment?

gabrielschulhof · 2021-10-01T15:14:57Z

unit-test/injectTestParams.js

+ *
+ *  @returns : list of files to compile by node-gyp
+ *  @param : none
+ *  @requires : picks `filter` parameter from process.env
+ *  This function is used as an utility method to inject a list of files to compile into binding.gyp
+ *
+ *


Same here. Do we need the first and trailing blank lines?

@gabrielschulhof agreed , removing blank lines

gabrielschulhof · 2021-10-01T15:15:28Z

unit-test/injectTestParams.js

+ * @param : none
+ * @requires : picks `filter` parameter from process.env
+ * This function is used as an utility method by the generateBindingCC step in binding.gyp
+ *


Blank line.

gabrielschulhof · 2021-10-01T15:16:06Z

unit-test/injectTestParams.js

+ *
+ *
+ *


Do we need these blank lines? Here and in all these comments?

gabrielschulhof · 2021-10-01T15:20:09Z

unit-test/spawnTask.js

+/*
+*  spawns a child process to run a given node.js script
+*/
+module.exports.runChildProcess = async function (scriptName, options) {


The function doesn't need to be async if it returns a promise.

It can be used from an async function without itself being marked as async.

@gabrielschulhof agreed

KevinEady

See comments.

KevinEady · 2021-10-15T12:52:52Z

test/index.js

@@ -87,7 +108,7 @@ if (napiVersion < 3) {
  testModules.splice(testModules.indexOf('version_management'), 1);
 }

-if (napiVersion < 4) {
+if (napiVersion < 4 && !filterConditionFiles.length) {


Why do we need to check for !filterConditionFiles.length here (and in the lines below as well)?

@KevinEady I am using the array filterConditionFiles as a flag to, Not apply versioning for filtered tests

@KevinEady suppose I specifically run tests for only 'asyncprogressqueueworker', I would expect the tests to run even if it does not satisfy node version criteria (the tests would fail which is ok, I can upgrade local node version and try again)

Please don't treat .length as a boolean!

KevinEady · 2021-10-15T12:55:05Z

unit-test/exceptions.js

+    threadsafe: 'threadSafe',
+    objectwrap: 'objectWrap'
+  },
+  exportNames: {
+    AsyncWorkerPersistent: 'PersistentAsyncWorker'
+  },
+  propertyNames: {
+    async_worker_persistent: 'persistentasyncworker',
+    objectwrap_constructor_exception: 'objectwrapConstructorException'


Did we discuss standardizing the test files themselves so this sort of manual checks aren't needed?

@KevinEady in some cases the exception from the standard form seems to be valid,
eg: persistentasyncworker is a good name to export from the test file async_worker_persistent

KevinEady · 2021-10-15T12:56:10Z

unit-test/test.js

+    const workingDir = path.join(__dirname, '../');
+    const relativeBuildPath = path.join('../', 'unit-test');
+    const buildPath = path.join(__dirname, './unit-test');
+    const envVars = { ...process.env, REL_BUILD_PATH: relativeBuildPath, BUILD_PATH: buildPath };


Is it needed to have both REL_BUILD_PATH and BUILD_PATH ...?

@KevinEady yes, the relative build path is required in a specific case in test/index.js where it determines the target build configuration (DEBUG or RELEASE)

the build path is necessary to import the test modules and run tests

I am still a little confused as to why you can't you also use the relative path instead of introducing more variables?

@KevinEady in

node-addon-api/test/index.js

Line 93 in 1f160cf

.readdirSync(path.join(__dirname, process.env.REL_BUILD_PATH || '', 'build'))

, a relative path from the test folder is required to a build folder :

-----> ./build test ---- -----> ../unit/ -> /build

that path reference is different from the path reference in

node-addon-api/test/common/index.js

Line 78 in 1f160cf

exports.runTest = async function (test, buildType, buildPathRoot = process.env.BUILD_PATH || '') {

where we need the complete root to a build path

KevinEady · 2021-10-15T12:56:56Z

unit-test/binding-file-template.js

+  // content.push("Object InitName(Env env);");
+  inits.forEach(init => content.push(init));
+
+  content.push('Object Init(Env env, Object exports) {');
+
+  // content.push("exports.Set(\"name\", InitName(env));");


Remove unnecessary comments

KevinEady · 2021-10-15T12:57:46Z

unit-test/generate-binding-cc.js

+generateBindingConfigurations().then(generateFileContent).then(writeToBindingFile);
+
+/**
+ *


Also at the end of this comment as well

deepakrkris · 2021-10-15T22:37:54Z

@gabrielschulhof @KevinEady I have responded to all comments, please let me know your feedback

mhdawson · 2021-10-25T15:43:13Z

@gabrielschulhof @KevinEady do you want to have another look before this lands?

gabrielschulhof · 2021-10-29T15:38:51Z

package.json

@@ -378,7 +378,9 @@
    "dev:incremental": "node test",
    "doc": "doxygen doc/Doxyfile",
    "lint": "eslint $(git diff --name-only refs/remotes/origin/main '**/*.js' | xargs) && node tools/clang-format",
-    "lint:fix": "node tools/clang-format --fix && eslint --fix $(git diff --cached --name-only '**/*.js' | xargs && git diff --name-only '**/*.js' | xargs)"
+    "lint:fix": "node tools/clang-format --fix && eslint --fix $(git diff --cached --name-only '**/*.js' | xargs && git diff --name-only '**/*.js' | xargs)",
+    "preunit": "filter=\"$npm_config_filter\" node-gyp rebuild -C unit-test",


OK. I understand. Still, I think for the simplicity of the command line it would be better to move this assignment into the JS file. So, like

// npm prepends "npm_config_" to all environment variables created from npm command line parameters. let filter = process.env.npm_config_filter;

gabrielschulhof · 2021-10-29T15:40:08Z

test/common/index.js

-exports.mustNotCall = function(msg) {
-  return function mustNotCall() {
+exports.mustNotCall = function (msg) {
+  return function mustNotCall () {


There are a lot of whitespace changes that inflate the diff. Are they the result of running a linter?

Yes, it's the result of running the linter ☺

deepakrkris · 2021-11-19T15:56:05Z

@gabrielschulhof I have changed the npm_config_filter refererence now.

#1078 (comment)

KevinEady · 2021-12-03T15:48:31Z

So, some of the addons don't run correctly in unit-test mode. I think we can tackle that at a later date. However, I think there is a bigger issue in that a failed unit test exits the npm run unit --filter=<filter> process successfully:

➜  node-addon-api git:(b398038) ✗ npm run unit --filter=addon ; echo "Return code: $?"


> [email protected] preunit /Users/kevineady/Documents/Projects/node-addon-api
> filter="$npm_config_filter" node-gyp rebuild -C unit-test

  ACTION Generating binding cc file generated/binding.cc
test modules to bind:  [ 'addon' ]
generated binding file  /Users/kevineady/Documents/Projects/node-addon-api/unit-test/generated/binding.cc 2021-12-03T15:40:16.308Z
  TOUCH Release/obj.target/generateBindingCC.stamp
  CXX(target) Release/obj.target/binding/generated/binding.o
  CXX(target) Release/obj.target/binding/../test/addon.o
  SOLINK_MODULE(target) Release/binding.node
  CXX(target) Release/obj.target/binding_noexcept/generated/binding.o
  CXX(target) Release/obj.target/binding_noexcept/../test/addon.o
  SOLINK_MODULE(target) Release/binding_noexcept.node
  CXX(target) Release/obj.target/binding_noexcept_maybe/generated/binding.o
  CXX(target) Release/obj.target/binding_noexcept_maybe/../test/addon.o
  SOLINK_MODULE(target) Release/binding_noexcept_maybe.node
  CXX(target) Release/obj.target/binding_swallowexcept/generated/binding.o
  CXX(target) Release/obj.target/binding_swallowexcept/../test/addon.o
  SOLINK_MODULE(target) Release/binding_swallowexcept.node
  CXX(target) Release/obj.target/binding_swallowexcept_noexcept/generated/binding.o
  CXX(target) Release/obj.target/binding_swallowexcept_noexcept/../test/addon.o
  SOLINK_MODULE(target) Release/binding_swallowexcept_noexcept.node

> [email protected] unit /Users/kevineady/Documents/Projects/node-addon-api
> filter="$npm_config_filter" node unit-test/test

Starting to run tests in  /Users/kevineady/Documents/Projects/node-addon-api/unit-test/unit-test 2021-12-03T15:40:22.515Z
napiVersion:5

Testing with Node-API Version '5'.

Starting test suite
 [ 'addon' ]

Running test 'addon'

error: dyld: lazy symbol binding failed: Symbol not found: __Z9InitAddonN4Napi3EnvE
  Referenced from: /Users/kevineady/Documents/Projects/node-addon-api/unit-test/build/Release/binding.node
  Expected in: flat namespace

dyld: Symbol not found: __Z9InitAddonN4Napi3EnvE
  Referenced from: /Users/kevineady/Documents/Projects/node-addon-api/unit-test
error: /build/Release/binding.node
  Expected in: flat namespace


error: Tests aborted with SIGABRT

child process exited with code 1
Completed running tests 2021-12-03T15:40:24.973Z
Return code: 0

I think this is a blocker for merging the PR, as the return value should signify if the test passes or not.

gabrielschulhof · 2021-12-03T16:38:09Z

@deepakrkris please also remove the aliasing of npm_config_filter to filter from the command lines in package.json.

gabrielschulhof

Marking as Request changes per @KevinEady's comment about returning success even on error. Please dismiss when the problem is fixed!

deepakrkris · 2021-12-10T15:47:10Z

@deepakrkris please also remove the aliasing of npm_config_filter to filter from the command lines in package.json.

@gabrielschulhof I have fixed this now

deepakrkris · 2021-12-10T15:47:53Z

Marking as Request changes per @KevinEady's comment about returning success even on error. Please dismiss when the problem is fixed!

@KevinEady @gabrielschulhof I have fixed this now

deepakrkris · 2021-12-10T15:49:28Z

@KevinEady I am able verify the fix as below

SFOC02CC4L3MD6T:node-addon-api t_rajad$ npm run unit --filter=addon ; echo "Return code: $?"

> [email protected] preunit /Users/t_rajad/projects/AA-NAPI/node-addon-api
> node-gyp rebuild -C unit-test

  ACTION Generating binding cc file generated/binding.cc
test modules to bind:  [ 'addon' ]
generated binding file  /Users/t_rajad/projects/AA-NAPI/node-addon-api/unit-test/generated/binding.cc 2021-12-10T15:48:28.973Z
  TOUCH Release/obj.target/generateBindingCC.stamp
  CXX(target) Release/obj.target/binding/generated/binding.o
  CXX(target) Release/obj.target/binding/../test/addon.o
  SOLINK_MODULE(target) Release/binding.node
  CXX(target) Release/obj.target/binding_noexcept/generated/binding.o
  CXX(target) Release/obj.target/binding_noexcept/../test/addon.o
  SOLINK_MODULE(target) Release/binding_noexcept.node
  CXX(target) Release/obj.target/binding_noexcept_maybe/generated/binding.o
  CXX(target) Release/obj.target/binding_noexcept_maybe/../test/addon.o
  SOLINK_MODULE(target) Release/binding_noexcept_maybe.node
  CXX(target) Release/obj.target/binding_swallowexcept/generated/binding.o
  CXX(target) Release/obj.target/binding_swallowexcept/../test/addon.o
  SOLINK_MODULE(target) Release/binding_swallowexcept.node
  CXX(target) Release/obj.target/binding_swallowexcept_noexcept/generated/binding.o
  CXX(target) Release/obj.target/binding_swallowexcept_noexcept/../test/addon.o
  SOLINK_MODULE(target) Release/binding_swallowexcept_noexcept.node

> [email protected] unit /Users/t_rajad/projects/AA-NAPI/node-addon-api
> node unit-test/test

Starting to run tests in  /Users/t_rajad/projects/AA-NAPI/node-addon-api/unit-test/unit-test 2021-12-10T15:48:38.701Z
napiVersion:7

Testing with Node-API Version '7'.

Starting test suite
 [ 'addon' ]
Running test 'addon'

TypeError: binding.addon.increment1 is not a function
    at test (/Users/t_rajad/projects/AA-NAPI/node-addon-api/test/addon.js:8:36)
    at Object.exports.runTest (/Users/t_rajad/projects/AA-NAPI/node-addon-api/test/common/index.js:88:27)
    at Object.<anonymous> (/Users/t_rajad/projects/AA-NAPI/node-addon-api/test/addon.js:5:38)
    at Module._compile (internal/modules/cjs/loader.js:1015:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1035:10)
    at Module.load (internal/modules/cjs/loader.js:879:32)
    at Function.Module._load (internal/modules/cjs/loader.js:724:14)
    at Module.require (internal/modules/cjs/loader.js:903:19)
    at require (internal/modules/cjs/helpers.js:74:18)
    at /Users/t_rajad/projects/AA-NAPI/node-addon-api/test/index.js:150:11

child process exited with code 1
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] unit: `node unit-test/test`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] unit script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/t_rajad/.npm/_logs/2021-12-10T15_48_39_018Z-debug.log
Return code: 1

KevinEady

LGTM! We should figure out why individual unit tests fail, but I think that can come at a different time.

I have verified the return code is now non-zero for failed unit tests:

➜  node-addon-api git:(bc7622e) ✗ npm run unit --filter=addon ; echo "Return code: $?"


> [email protected] preunit /Users/kevineady/Documents/Projects/node-addon-api
> node-gyp rebuild -C unit-test

  ACTION Generating binding cc file generated/binding.cc
test modules to bind:  [ 'addon' ]
generated binding file  /Users/kevineady/Documents/Projects/node-addon-api/unit-test/generated/binding.cc 2021-12-10T16:03:38.620Z
  TOUCH Release/obj.target/generateBindingCC.stamp
  CXX(target) Release/obj.target/binding/generated/binding.o
  CXX(target) Release/obj.target/binding/../test/addon.o
  SOLINK_MODULE(target) Release/binding.node
  CXX(target) Release/obj.target/binding_noexcept/generated/binding.o
  CXX(target) Release/obj.target/binding_noexcept/../test/addon.o
  SOLINK_MODULE(target) Release/binding_noexcept.node
  CXX(target) Release/obj.target/binding_noexcept_maybe/generated/binding.o
  CXX(target) Release/obj.target/binding_noexcept_maybe/../test/addon.o
  SOLINK_MODULE(target) Release/binding_noexcept_maybe.node
  CXX(target) Release/obj.target/binding_swallowexcept/generated/binding.o
  CXX(target) Release/obj.target/binding_swallowexcept/../test/addon.o
  SOLINK_MODULE(target) Release/binding_swallowexcept.node
  CXX(target) Release/obj.target/binding_swallowexcept_noexcept/generated/binding.o
  CXX(target) Release/obj.target/binding_swallowexcept_noexcept/../test/addon.o
  SOLINK_MODULE(target) Release/binding_swallowexcept_noexcept.node

> [email protected] unit /Users/kevineady/Documents/Projects/node-addon-api
> node unit-test/test

Starting to run tests in  /Users/kevineady/Documents/Projects/node-addon-api/unit-test/unit-test 2021-12-10T16:03:49.692Z
napiVersion:5

Testing with Node-API Version '5'.

Starting test suite
 [ 'addon' ]

Running test 'addon'

error: dyld: lazy symbol binding failed: Symbol not found: __Z9InitAddonN4Napi3EnvE
  Referenced from: /Users/kevineady/Documents/Projects/node-addon-api/unit-test/build/Release/binding.node
  Expected in: flat namespace

dyld: Symbol not found: __Z9InitAddonN4Napi3EnvE
  Referenced from: /Users/kevineady/Documents/Projects/node-addon-api/unit-test/build/Release/binding.node
  Expected in: flat namespace


error: Tests aborted with SIGABRT

child process exited with code 1
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] unit: `node unit-test/test`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] unit script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/kevineady/.npm/_logs/2021-12-10T16_03_50_458Z-debug.log

Return code: 1

deepakrkris · 2021-12-17T15:56:32Z

@KevinEady I cloned the repo (from my fork) freshly and the unit test with filter=addon runs good


> [email protected] preunit /Users/t_rajad/projects/AA-NAPI/node-addon-api
> node-gyp rebuild -C unit-test

  ACTION Generating binding cc file generated/binding.cc
test modules to bind:  [ 'addon' ]
generated binding file  /Users/t_rajad/projects/AA-NAPI/node-addon-api/unit-test/generated/binding.cc 2021-12-17T15:54:59.530Z
  TOUCH Release/obj.target/generateBindingCC.stamp
  CXX(target) Release/obj.target/binding/generated/binding.o
  CXX(target) Release/obj.target/binding/../test/addon.o
  SOLINK_MODULE(target) Release/binding.node
  CXX(target) Release/obj.target/binding_noexcept/generated/binding.o
  CXX(target) Release/obj.target/binding_noexcept/../test/addon.o
  SOLINK_MODULE(target) Release/binding_noexcept.node
  CXX(target) Release/obj.target/binding_noexcept_maybe/generated/binding.o
  CXX(target) Release/obj.target/binding_noexcept_maybe/../test/addon.o
  SOLINK_MODULE(target) Release/binding_noexcept_maybe.node
  CXX(target) Release/obj.target/binding_swallowexcept/generated/binding.o
  CXX(target) Release/obj.target/binding_swallowexcept/../test/addon.o
  SOLINK_MODULE(target) Release/binding_swallowexcept.node
  CXX(target) Release/obj.target/binding_swallowexcept_noexcept/generated/binding.o
  CXX(target) Release/obj.target/binding_swallowexcept_noexcept/../test/addon.o
  SOLINK_MODULE(target) Release/binding_swallowexcept_noexcept.node

> [email protected] unit /Users/t_rajad/projects/AA-NAPI/node-addon-api
> node unit-test/test

Starting to run tests in  /Users/t_rajad/projects/AA-NAPI/node-addon-api/unit-test/unit-test 2021-12-17T15:55:08.468Z
napiVersion:7

Testing with Node-API Version '7'.

Starting test suite
 [ 'addon' ]
Running test 'addon'


All tests passed!

child process exited with code 0
SFOC02CC4L3MD6T:node-addon-api t_rajad$

deepakrkris · 2021-12-17T15:58:40Z

@KevinEady I cloned the forked branch and worked on it. Is there another way you have cloned the repo like say cloning the root repo and then checking out to a forked branch ? I can try that as well.

gabrielschulhof · 2022-01-14T08:01:07Z

test/index.js

+let filterCondition = process.env.npm_config_filter || '';
+let filterConditionFiles = [];
+
+if (process.env.npm_config_filter !== null && process.env.npm_config_filter !== undefined && process.env.npm_config_filter.trim() !== '') {


Why not if (filterCondition != null && ...), accounting for the fact that the default is '' and not null?

gabrielschulhof · 2022-01-14T08:02:59Z

test/index.js

+
+if (process.env.npm_config_filter !== null && process.env.npm_config_filter !== undefined && process.env.npm_config_filter.trim() !== '') {
+  filterCondition = require('../unit-test/matchModules').matchWildCards(process.env.npm_config_filter);
+  filterConditionFiles = filterCondition.split(' ').length ? filterCondition.split(' ') : [filterCondition];


You should make this check explicitly > 0.

gabrielschulhof · 2022-01-14T08:03:29Z

test/index.js

+function checkFilterCondition (fileName, parsedFilepath) {
+  let result = false;
+
+  if (!filterConditionFiles.length) result = true;


Same here (> 0).

gabrielschulhof · 2022-01-14T08:04:29Z

test/index.js

@@ -87,7 +108,7 @@ if (napiVersion < 3) {
  testModules.splice(testModules.indexOf('version_management'), 1);
 }

-if (napiVersion < 4) {
+if (napiVersion < 4 && !filterConditionFiles.length) {


Please don't treat .length as a boolean!

mhdawson

LGTM

mhdawson · 2022-01-14T19:51:29Z

@deepakrkris I see there are a few final comments from @gabrielschulhof . If you can address those I'll land.

mhdawson · 2022-01-14T19:52:15Z

@deepakrkris if you can squash down to 1 commit that would also make it much easier for me to land and it seems individual commits have conflicts.

mhdawson · 2022-01-14T20:14:21Z

@deepakrkris ran the ci jobs https://ci.nodejs.org/job/node-test-node-addon-api-new/ for latest branch and 12.x and all seems ok so we should be good once you update the last few comments. If you can email me once you do that that will help me get to sooner.

deepakrkris · 2022-01-14T20:54:16Z

@deepakrkris I see there are a few final comments from @gabrielschulhof . If you can address those I'll land.

@mhdawson sure, I will fix the comments and ping here in a few hours, thank you

deepakrkris · 2022-01-28T16:17:54Z

@deepakrkris if you can squash down to 1 commit that would also make it much easier for me to land and it seems individual commits have conflicts.

@mhdawson I have fixed comments from Gabriel and squashed the commits

deepakrkris · 2022-01-28T16:42:53Z

#1078 (comment) , @gabrielschulhof agreed, fixed the comments now

mhdawson · 2022-01-31T22:00:42Z

Landed as 744c8d2

Document all possible filter conditions added in nodejs#1078

* Update Readme for filter conditions in unit tests Document all possible filter conditions added in #1078 PR-URL:#1199 Reviewed-By: Michael Dawson <[email protected]>

* Update Readme for filter conditions in unit tests Document all possible filter conditions added in nodejs/node-addon-api#1078 PR-URL:nodejs/node-addon-api#1199 Reviewed-By: Michael Dawson <[email protected]>

deepakrkris mentioned this pull request Sep 23, 2021

[WIP] enable unit-tests #1030

Closed

deepakrkris force-pushed the filter-tests branch from a43bd0c to 7190df5 Compare September 24, 2021 02:18

gabrielschulhof reviewed Oct 1, 2021

View reviewed changes

gabrielschulhof mentioned this pull request Oct 1, 2021

Adding unit tests for FunctionReference class #1035

Closed

deepakrkris force-pushed the filter-tests branch 4 times, most recently from 4bc30b5 to defc460 Compare October 15, 2021 07:10

KevinEady reviewed Oct 15, 2021

View reviewed changes

deepakrkris force-pushed the filter-tests branch from 29068da to 2d70c10 Compare October 15, 2021 22:36

gabrielschulhof reviewed Nov 5, 2021

View reviewed changes

gabrielschulhof requested changes Dec 3, 2021

View reviewed changes

KevinEady self-requested a review December 10, 2021 16:04

KevinEady approved these changes Dec 10, 2021

View reviewed changes

deepakrkris requested a review from gabrielschulhof December 10, 2021 16:08

gabrielschulhof approved these changes Jan 14, 2022

View reviewed changes

mhdawson approved these changes Jan 14, 2022

View reviewed changes

deepakrkris force-pushed the filter-tests branch from 8dcdcc6 to 6603f37 Compare January 28, 2022 16:11

run specific test cases with filter conditions

17aaeca

deepakrkris force-pushed the filter-tests branch from 6603f37 to 17aaeca Compare January 28, 2022 16:16

mhdawson closed this Jan 31, 2022

deepakrkris added a commit to deepakrkris/node-addon-api that referenced this pull request Aug 19, 2022

Update Readme for filter conditions in unit tests

5550637

Document all possible filter conditions added in nodejs#1078

deepakrkris mentioned this pull request Aug 19, 2022

Update Readme for filter conditions in unit tests #1199

Merged

enable unit tests #1078

enable unit tests #1078

Conversation

deepakrkris commented Sep 23, 2021 • edited Loading

Enable running tests with specific filter conditions:

Example:

Wildcards are also possible:

Example:

Multiple filter conditions are also allowed

Example:

deepakrkris commented Sep 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KevinEady left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepakrkris Oct 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepakrkris commented Oct 15, 2021

mhdawson commented Oct 25, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepakrkris commented Nov 19, 2021

KevinEady commented Dec 3, 2021

gabrielschulhof commented Dec 3, 2021

gabrielschulhof left a comment • edited Loading

Choose a reason for hiding this comment

deepakrkris commented Dec 10, 2021

deepakrkris commented Dec 10, 2021

deepakrkris commented Dec 10, 2021

KevinEady left a comment

Choose a reason for hiding this comment

deepakrkris commented Dec 17, 2021

deepakrkris commented Dec 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhdawson left a comment

Choose a reason for hiding this comment

mhdawson commented Jan 14, 2022

mhdawson commented Jan 14, 2022

deepakrkris commented Sep 23, 2021 •

edited

Loading

deepakrkris Oct 29, 2021 •

edited

Loading

gabrielschulhof left a comment •

edited

Loading

deepakrkris commented Jan 28, 2022 •

edited

Loading