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

Make tests work on all Node versions #44

Merged
merged 2 commits into from
May 23, 2017
Merged

Conversation

jasongin
Copy link
Member

@jasongin jasongin commented May 20, 2017

  • Make GC tests (arraybuffer, buffer, external) async, to account for different GC behavior with different versions of V8 and ChakraCore, similar to test: Make N-API weak-ref GC tests asynchronous node#13121
  • In test/index.js, use the --napi-modules and --expose-gc command-line flags automatically
  • Add missing entry for object tests in index.js.
  • Remove check for writable attribute on accessor property descriptors; it should not be there according to the JS spec.
  • Remove the explicit dependency on node-gyp in package.json. (NPM carries its own copy of node-gyp.)

@jasongin jasongin force-pushed the async-tests branch 2 times, most recently from debda2e to 3bb8676 Compare May 21, 2017 16:56
@jasongin jasongin mentioned this pull request May 21, 2017
 - Make GC tests (arraybuffer, buffer, external) async, to account for
   different GC behavior with different versions of V8 and ChakraCore,
   similar to nodejs/node#13121
 - In test/index.js, use the --napi-modules and --expose-gc
   command-line flags automatically.
 - Add missing entry for object tests in index.js.
 - Remove check for writable attribute on accessor property
   descriptors; it should not be there according to the JS spec.
 - Remove the explicit dependency on node-gyp in package.json.
   (NPM carries its own copy of node-gyp.)
Improper rethrow of an Error caught by reference caused a double napi_ref delete, which failed in release builds of Node-ChakraCore.
@jasongin jasongin merged commit e4f6030 into nodejs:master May 23, 2017
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

2 participants