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

[WIP] enable unit-tests #1030

Closed
wants to merge 57 commits into from
Closed

Conversation

deepakrkris
Copy link
Contributor

No description provided.

@deepakrkris deepakrkris changed the title enable unit-tests [WIP] enable unit-tests Jul 30, 2021
Copy link
Contributor

@KevinEady KevinEady left a comment

Choose a reason for hiding this comment

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

Don't want to lose track of some discussion we had from our previous meetings:

  1. Standardizing the test + object names so you wouldn't need the testFunctionsMap
  2. Merging your filtering capabilities with the existing 'npm test' command. The CI would run it with no filters, and users locally can optionally provide the filters to run at testing time.

@deepakrkris
Copy link
Contributor Author

deepakrkris commented Aug 27, 2021

Don't want to lose track of some discussion we had from our previous meetings:

  1. Standardizing the test + object names so you wouldn't need the testFunctionsMap
  2. Merging your filtering capabilities with the existing 'npm test' command. The CI would run it with no filters, and users locally can optionally provide the filters to run at testing time.

@KevinEady thanks for summarizing , I have opened PR
#1055 and
#1056
for the first requirement

addaleax and others added 22 commits September 23, 2021 10:59
Not sure how this ever made its way into the API, but people should *never* use this.
PR-URL: nodejs#937
Reviewed-By: Michael Dawson <midawson@redhat.com>
* doc: fix tab indent
Added windows-2016 as virtual environment.

PR-URL: nodejs#948
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Also fixed lint errors.

Fixes: nodejs#952
PR-URL: nodejs#953
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Done by running:
sed -i "s/N-API/Node-API/g" doc/*
sed -i "s/N-API/Node-API/g" README.md CONTRIBUTING.md

Fixes: nodejs#950
PR-URL: nodejs#951
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Currently, deprecation notices are always right between two function
signatures and it's virtually impossible to be certain whether they
refer to the previous signature or the next signature.

PR-URL: nodejs#942
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
PR-URL: nodejs#939
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
PR-URL: nodejs#955
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
PR-URL: nodejs#974
Reviewed-By: Michael Dawson <midawson@redhat.com>
PR-URL: nodejs#973
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
PR-URL: nodejs#976
Reviewed-By: Michael Dawson <midawson@redhat.com>
NickNaso and others added 26 commits September 23, 2021 10:59
PR-URL: nodejs#1005
Reviewed-By: Michael Dawson <midawson@redhat.com>
Fixes: nodejs#1007

PR-URL: nodejs#1009
Reviewed-By: Michael Dawson <midawson@redhat.com>
When terminating an environment (e.g., by calling worker.terminate),
napi_throw, which is called by Error::ThrowAsJavaScriptException,
returns napi_pending_exception, which is then incorrectly treated as
a fatal error resulting in a crash.

PR-URL: nodejs#975
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
PR-URL: nodejs#1004
Reviewed-By: Michael Dawson <midawson@redhat.com>
Fixes: nodejs#1011

PR-URL: nodejs#1013
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
PR-URL: nodejs#1015
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Kevin Eady <kevin.c.eady@gmail.com>
PR-URL: nodejs#972
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
Throw an exception when receiving a null pointer for the `char *` and
`char16_t *` overloads of `String::New` that looks identical to an
error that core would have thrown under the circumstances
(`napi_invalid_arg`).

Also, rename the test methods to conform with our naming convention.

PR-URL: nodejs#1019
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
fixes: nodejs#1022

The objectwrap_worker_thread and symbol tests
were not waiting for the test to complete before the
subsequent tests were started. This caused intermitted
crashes in the CI.

Updated both tests so that they complete before the
next test runs.

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: nodejs#1024
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Add CleanupHook support to Env

PR-URL: nodejs#1014
Reviewed-By: Michael Dawson <midawson@redhat.com>
PR-URL: nodejs#927
Reviewed-By: Michael Dawson <midawson@redhat.com>
PR-URL: nodejs#1061
Reviewed-By: Michael Dawson <midawson@redhat.com>
* doc: fixed doc about how to enable C++ exceptions.

PR-URL: nodejs#1059
Reviewed-By: Michael Dawson <midawson@redhat.com>
- fix error reported about possible use of
  un-initialized variable from newer compiler

Signed-off-by: Michael Dawson <mdawson@devrus.com>
- change all unit test file names to snake case
  this helps in parsing file names to tokens and infer metadata
  like export initializers

- all other changes other than file names is due to linter errors

PR-URL: nodejs#1056
Reviewed-By: Michael Dawson <midawson@redhat.com
Reviewed-By: NickNaso <nicoladelgobbo@gmail.com>
PR-URL: nodejs#1055
Reviewed-By: Michael Dawson <midawson@redhat.com>
I guess if these were broken in practice, V8/Node.js itself would also
experience difficulties with it, but there’s no real reason not to use
the proper alternatives.

PR-URL: nodejs#1070
Reviewed-By: Michael Dawson <midawson@redhat.com>
Function::New fails to compile when given a move-only functor. For
example, when constructing a callback function for Promise#then, a
lambda might capture an ObjectReference. Creating a Function for such a
lambda results in a compilation error.

Tweak Function::New to work with move-only functors.

For existing users of Function::New, this commit should not change
behavior.
* lint: add eslint based on config-semistandard
@deepakrkris
Copy link
Contributor Author

@KevinEady @mhdawson I have opened a new PR #1078 and closed this branch , had troubles with rebasing

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