test: Improvement for testing of execSync #9133
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PR-URL: nodejs#8588 Reviewed-By: Yosuke Furukawa <yosuke.furukawa@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Make it clear that atime and mtime should be in seconds. PR-URL: nodejs#8651 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
PR-URL: nodejs#8628 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Changed var to const PR-URL: nodejs#8599 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Fixes: nodejs#8426 PR-URL: nodejs#8502 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: nodejs#8578 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Changed vars to consts and lets, assert.equals to assert.strictEquals and added common.mustCall around callbacks. Switched to arrow functions. PR-URL: nodejs#8616 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Replace equal with strictEqual, use const instead of var and improve test with use of assert.notStrictEqual PR-URL: nodejs#8600 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Moves inflateSetDictionary right after inflateInit2 when mode is INFLATERAW, since without the wrapper in appears zlib won't return Z_NEED_DICT as it would otherwise, and will thus attempt inflating without the dictionary, leading to an error. Fixes: nodejs#8507 PR-URL: nodejs#8512 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
Uses const where possible, removes inaccurate comments, prefers strictEqual where possible, ensures functions with assertions are called and enures the inflater has correct encoding set. PR-URL: nodejs#8512 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
Changed vars to const / let, functions to arrow functions and a mustCall where appropriate. PR-URL: nodejs#8581 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
change var to const/let where appropriate use strictEqual instead of equal call toString on buffers in strictEqual asserts use common.mustCall on callbacks containing asserts PR-URL: nodejs#8648 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
PR-URL: nodejs#8622 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com> Reviewed-By: Jackson Tian <shvyo1987@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
This commit prevents thrown JavaScript exceptions from crashing the process in node_contextify's CopyProperties() function. Fixes: nodejs#8537 PR-URL: nodejs#8649 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
Refactored the code to latest standards, where all var is changed to const, functions are changed to arrow functions and assert.equal chaned to assert.strictEqual PR-URL: nodejs#8582 Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
PR-URL: nodejs#8620 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jackson Tian <shvyo1987@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit adds better handling of exceptional array formats passed to dns.setServers(). Prior to this commit, the input array was validated using map(), which preserves holes, allowing them to be passed to c-ares, crashing Node. This commit replaces map() with forEach(), which skips holes. Fixes: nodejs#8538 PR-URL: nodejs#8567 Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#8676 Reviewed-By: Rich Trott <rtrott@gmail.com>
If calling `https.request()` with `options.headers.host` defined and `options.servername` undefined, `https.Agent.createSocket` mutates connection `options` after `https.Agent.addRequest` has created empty socket pool array with mismatching connection name. This results in two socket pool arrays being created and only the last one gets eventually deleted by `removeSocket` - causing a memory leak. This commit fixes the leak by making sure that `addRequest` does the same modifications to `options` object as the `createSocket`. `createSocket` is intentionally left unmodified to prevent userland regressions. Test case included. PR-URL: nodejs#8647 Fixes: nodejs#6687 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jackson Tian <shvyo1987@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Moved from var to const. Moved from .equal to .strictEqual. Added more checks about the type of the returned values. PR-URL: nodejs#8606 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
PR-URL: nodejs#8643 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@keybase.io>
PR-URL: nodejs#8696 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Specify the plugin name as 'remark-lint/', as new remark-cli refused to find it when cwd is different from the directory where node_modules are put. Trailing slash fixes this, as the plugin name gets treated as a path, so `require` works on it. Explicitly specify the list of rules we want to enable, as the new remark-lint is opt-in and doesn't come with any rules by default. Reorder the rules alphabetically. PR-URL: nodejs#8666 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
Change `Malloc()/Calloc()` so that size zero does not return a null pointer, consistent with prior behavior. Fixes: nodejs#8571 PR-URL: nodejs#8572 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@keybase.io> Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Pick up latest commit from the 5.4-lkgr branch. deps: edit V8 gitignore to allow trace event copy deps: update V8 trace event to 315bf1e2d45be7d53346c31cfcc37424a32c30c8 deps: edit V8 gitignore to allow gtest_prod.h copy deps: update V8 gtest to 6f8a66431cb592dad629028a50b3dd418a408c87 PR-URL: nodejs#8317 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
The location of various gypfiles has changed in V8 5.2. PR-URL: nodejs#8317 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
v8.gyp expects this to be defined by the embedder PR-URL: nodejs#8317 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Michaël Zasso <targos@protonmail.com>
V8 now depends on C++11 runtime features. On OSX this requires us to link against the libc++ library rather than the deprecated default that is provided with -mmacosx-version-min=10.7. PR-URL: nodejs#8317 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Michaël Zasso <targos@protonmail.com>
V8 needs it for case conversion. Ref: https://codereview.chromium.org/1812673005 PR-URL: nodejs#8317 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
mkpeephole is a binary that gets generated and run at V8 build time. On cross-compilation builds separate build toolchains are required. Similar to want_separate_host_toolset we default to disabled. PR-URL: nodejs#8317 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Michaël Zasso <targos@protonmail.com>
This replaces TickObject with an object literal. This offers performance improvements of up to ~20%. PR-URL: nodejs#8932 Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Trevor Norris <trev.norris@gmail.com>
* var to const * add check that expected error is ENOENT * indexOf() to includes() PR-URL: nodejs#8999 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
PR-URL: nodejs#9089 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Wrapped the timer into class to ensure it is cleaned up properly. PR-URL: nodejs#8870 Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Adds documentation and explicit reasons on why the GitHub web interface button is not used. This was explained in the referenced issue by @thealphanerd. Fixes: nodejs#8893 PR-URL: nodejs#9044 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Adds verbose reasons to the documentation on why the Reviewed-By metadata on a pull request is important. This was loosely mentioned as an issue in the referenced issue below, and answered by @addaleax. Ref: nodejs#8893 PR-URL: nodejs#9044 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
tick-processor-base.js is a module used by three other tests. It is not a test fixture so move it out of the fixture directory. (One downside to having it in the fixture directory is that fixture code is not currently linted.) It is possible that the code in tick-processor-base.js should be integrated into common.js. This can potentially happen subsequently (and might make a reasonable good first contribution for a new contributor). PR-URL: nodejs#9022 Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Highlight deprecated API methods/properties in "Table of Contents" for increasing understandability. Adapted code to eslint standards. PR-URL: nodejs#7189 Fixes: nodejs/nodejs.org#772 Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
The config.gypi target has a recipe that uses the control function error to report if the config.gypi file is missing or if it is stale (the configure file was updated which is a prerequisite of this rule). GNU make has two phases, immediate and deferred. During the first phase it will expand any variables or functions as the makefile is parsed. The recipe in this case is a shell if statement, which is a deferred construct. But the control function $(error) is an immediate construct which will cause the makefile processing to stop during the first phase of the Make process. If I understand this correctly the only possible outcome of this rule is the "Stale config.gypi, please re-run ./configure" message which will be done in the first phase and then exit. The shell condition will not be considered. So it will never report that the config.gypi is missing. bnoordhuis suggested that we simply change this into a single error message: "Missing or stale config.gypi, please run configure" PR-URL: nodejs#9053 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
`end` MUST always be emitted **before** `close`. However, if a handle will invoke `uv_close_cb` immediately, or in the same JS tick - `close` may be emitted first. PR-URL: nodejs#9066 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
PR-URL: nodejs#8988 Reviewed-By: James M Snell <jasnell@gmail.com>
This adds a new ESLint tool to check for let declarations within the for, forIn, forOf expressions. Fixes: nodejs#9045 Ref: nodejs#8873 PR-URL: nodejs#9049 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This commit fixes a regression introduced in 0ed8839 that caused additional queued immediate callbacks to be ignored if `clearImmediate(immediate)` was called within the callback for `immediate`. PR-URL: nodejs#9086 Fixes: nodejs#9084 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
2a4b068 introduced a regression in where checking `instanceof` would fail for `Writable` subclasses inside the subclass constructor, i.e. before `Writable()` was called. Also, calling `null instanceof Writable` or `undefined instanceof Writable` would fail due to accessing the `_writableState` property of the target object. This fixes these problems. PR-URL: nodejs#9088 Ref: nodejs#8834 (comment) Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
Commit 782620f added the define only when building with the bundled zlib. Using a shared zlib results in build breakage: ../src/inspector_agent.cc:179:16: error: assigning to 'Bytef *' (aka 'unsigned char *') from incompatible type 'const uint8_t *' (aka 'const unsigned char *') strm.next_in = PROTOCOL_JSON + 3; ^ ~~~~~~~~~~~~~~~~~ 1 error generated. PR-URL: nodejs#9077 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
* build: Fix building with shared zlib. (Bradley T. Hughes) [nodejs#9077](nodejs#9077) * stream: fix `Writable` subclass instanceof checks (Anna Henningsen) [nodejs#9088](nodejs#9088) * timers: fix regression with clearImmediate() (Brian White) [nodejs#9086](nodejs#9086) PR-URL: nodejs#9104
Ref: nodejs#8913 PR-URL: nodejs#8993 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Small refactoring to make contextify more readable. Remove auto and inline FromJust(). Simplify if statement. PR-URL: nodejs#8909 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
The documentation erroneously described the errno property as an alias for the code property, but that is not the case in the implementation. errno is the error code of the error as a number, and code is the error code of the error as a string. PR-URL: nodejs#9007 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Ref: nodejs#8913 PR-URL: nodejs#9047 Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Commit fdca79f ("test: enable addons test to pass with debug build") enabled the addons tests to pass when the build type is of type debug (configure --debug). test/addons/node-module-version/test.js was recently added and expects the the build type to be of type Release (like most of the others until recently). This commit allows this test to pass when the build type if of type debug. PR-URL: nodejs#9093 Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
In branch v4.x-staging, the error message is different:
('spawnSync exit -1 ENOENT' instead of 'spawnSync bad_shell ENOENT').
Since we care that ENOENT should be thrown, the regexp for ENOENT is enough.
Signed-off-by: Robert Chiras <robert.chiras@intel.com>
mscdex
added
the
child_process
|
/cc @Trott |
|
Can you please target this at the |
|
Change is fine if targeted at v4.x-staging branch like @cjihrig requests. Let's leave this change off master, though. It's not enough to just check the code because error message changes are semver-major in Node.js (for now at least, and for better or for worse). So we do want tests to blow up when messages change. |
|
This patch won't apply on v4.x-staging, so I ammended the initial patch and created a new pull request: #9152 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
make -j8 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
test
Description of change
In branch v4.x-staging, the error message is different:
('spawnSync exit -1 ENOENT' instead of 'spawnSync bad_shell ENOENT').
Since we care that ENOENT should be thrown, the regexp for ENOENT is enough.
Signed-off-by: Robert Chiras robert.chiras@intel.com