Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: added test case to check the addon building process. #808

Closed
wants to merge 6 commits into from
Closed

test: added test case to check the addon building process. #808

wants to merge 6 commits into from

Conversation

NickNaso
Copy link
Member

@NickNaso NickNaso commented Sep 4, 2020

This PR complete the work done on PR #766.
The PR introduce some test to check to build of an addon.
Refs:

lovell and others added 4 commits July 14, 2020 13:34
Deprecate use of `include` in an gyp array context, which
happens to work when paths are absolute, but can fail on Windows
when paths are relative and a gyp file contains multiple entries
in its `include_dirs` directive.

This change corrects documentation and tooling, adds support for
relative paths (e.g. those containing whitespace) in a backwards
compatible manner and makes the approach holistically consistent
with that used by nan.
@NickNaso NickNaso changed the title test: added test case for che test: added test case to check the addon building process. Sep 4, 2020
@NickNaso NickNaso mentioned this pull request Sep 4, 2020
6 tasks
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mhdawson
Copy link
Member

mhdawson commented Sep 4, 2020

@NickNaso there seem to be failures on 10.x for windows: https://ci.nodejs.org/view/x%20-%20Abi%20stable%20module%20API/job/node-test-node-addon-api-new/2728/nodes=win-vs2017/

and it seems to fail with most platforms on 15.x -> https://ci.nodejs.org/view/x%20-%20Abi%20stable%20module%20API/job/node-test-node-addon-api-new/2727/

What is see when I try to run locally is:

> node test

Testing with N-API Version '6'.
Starting test suite

Running test 'addon_build'
Error: Command failed: npm install
(node:105097) [DEP0139] DeprecationWarning: Calling process.umask() with no arguments is prone to race conditions and is a potential security vulnerability.
(Use `node --trace-deprecation ...` to show where the warning was created)
npm WARN npm npm does not support Node.js v15.0.0-pre
npm WARN npm You should probably upgrade to a newer version of node as we
npm WARN npm can't make any promises that npm will work with this version.
npm WARN npm Supported releases of Node.js are the latest release of 4, 6, 7, 8.
npm WARN npm You can find the latest version at https://nodejs.org/
npm ERR! cb.apply is not a function

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/mhdawson/.npm/_logs/2020-09-04T20_13_42_420Z-debug.log

    at ChildProcess.exithandler (child_process.js:308:12)
    at ChildProcess.emit (events.js:314:20)
    at maybeClose (internal/child_process.js:1051:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:287:5) {
  killed: false,
  code: 1,
  signal: null,
  cmd: 'npm install',
  stdout: '',
  stderr: '(node:105097) [DEP0139] DeprecationWarning: Calling process.umask() with no arguments is prone to race conditions and is a potential security vulnerability.\n' +
    '(Use `node --trace-deprecation ...` to show where the warning was created)\n' +
    'npm WARN npm npm does not support Node.js v15.0.0-pre\n' +
    'npm WARN npm You should probably upgrade to a newer version of node as we\n' +
    "npm WARN npm can't make any promises that npm will work with this version.\n" +
    'npm WARN npm Supported releases of Node.js are the latest release of 4, 6, 7, 8.\n' +
    'npm WARN npm You can find the latest version at https://nodejs.org/\n' +
    'npm ERR! cb.apply is not a function\n' +
    '\n' +
    'npm ERR! A complete log of this run can be found in:\n' +
    'npm ERR!     /home/mhdawson/.npm/_logs/2020-09-04T20_13_42_420Z-debug.log\n'
}
npm ERR! Test failed.  See above for more details.

@NickNaso
Copy link
Member Author

NickNaso commented Sep 7, 2020

@NickNaso
Copy link
Member Author

NickNaso commented Sep 8, 2020

CI:

Version Job Status
v15.x https://ci.nodejs.org/job/node-test-node-addon-api-new/2748
v14.x https://ci.nodejs.org/job/node-test-node-addon-api-new/2747/
v12.x https://ci.nodejs.org/job/node-test-node-addon-api-new/2746/
v10.x https://ci.nodejs.org/job/node-test-node-addon-api-new/2745/

Now the test works fine on local machine for Node.js 15.x. I'm experimenting a problem on CI:
ci-problem
It seems that the link used to get the nightly verson of Node.js does not exists.

I tried on my local machine on both Linux and WIndows and it works removing some checks on the test. It was necessary because until today npm uses process.umask() without arguments that now is deprecated and produces a warning that invalidate the test.
Refs:
npm/cli#1103

@mhdawson
Copy link
Member

mhdawson commented Sep 9, 2020

@NickNaso thanks, I assume this is now ready to land ?

@NickNaso
Copy link
Member Author

mhdawson pushed a commit that referenced this pull request Sep 10, 2020
Refs: #766
PR-URL: #808
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@mhdawson
Copy link
Member

Landed as 6562e6b

@mhdawson mhdawson closed this Sep 10, 2020
Superlokkus pushed a commit to Superlokkus/node-addon-api that referenced this pull request Nov 20, 2020
Refs: nodejs#766
PR-URL: nodejs#808
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this pull request Aug 24, 2022
Refs: nodejs/node-addon-api#766
PR-URL: nodejs/node-addon-api#808
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this pull request Aug 26, 2022
Refs: nodejs/node-addon-api#766
PR-URL: nodejs/node-addon-api#808
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this pull request Sep 19, 2022
Refs: nodejs/node-addon-api#766
PR-URL: nodejs/node-addon-api#808
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
austinli64 added a commit to austinli64/node-addon-api that referenced this pull request May 9, 2023
Refs: nodejs/node-addon-api#766
PR-URL: nodejs/node-addon-api#808
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023
Refs: nodejs/node-addon-api#766
PR-URL: nodejs/node-addon-api#808
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants