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

Replace test-buffer-bindingobj-no-zerofill.js by an addon test #22547

Closed
starkwang opened this issue Aug 27, 2018 · 3 comments
Closed

Replace test-buffer-bindingobj-no-zerofill.js by an addon test #22547

starkwang opened this issue Aug 27, 2018 · 3 comments
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. test Issues and PRs related to the tests.

Comments

@starkwang
Copy link
Contributor

starkwang commented Aug 27, 2018

Now the test/parallel/test-buffer-bindingobj-no-zerofill.js uses monkey-patches to simulate the situation when setupBufferJS() has an undefined zeroFill.

// Load from file system because internal buffer is already loaded and we're
// testing code that runs on first load only.
// Do not move this require() to top of file. It is important that
// `require('internal/buffer').setupBufferJS` be monkey-patched before this
// runs.
const monkeyPatchedBuffer = require('../../lib/buffer');

But the monkey-patches are little bit tricky and crash the tests when we move buffer module to internalBinding (#22370 (comment))

We need replace it by a C++ addon test now. See conversation at 27e84ddd4e1#r19182129 and #11706.

/cc @Trott @bnoordhuis

@starkwang starkwang added help wanted Issues that need assistance from volunteers or PRs that need help to proceed. c++ Issues and PRs that require attention from people who are familiar with C++. test Issues and PRs related to the tests. labels Aug 27, 2018
@vsnehil92
Copy link
Contributor

@starkwang please, may you mentor me to fix this issue?

@starkwang
Copy link
Contributor Author

@vsnehil92 It would be nice if you want to take this issue. Feel free to open a PR : )

@apapirovski
Copy link
Member

This has been addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. test Issues and PRs related to the tests.
Projects
None yet
Development

No branches or pull requests

3 participants