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.

Sign up for GitHub

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

Jump to bottom

[v10.x backport] test: refactor common.ddCommand() #23630

Closed
wants to merge 16 commits into from
Closed

[v10.x backport] test: refactor common.ddCommand() #23630

wants to merge 16 commits into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Oct 12, 2018

backport of #23411

@addaleax

@nodejs-github-bot
Copy link
Collaborator

@Trott build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1219/pipeline

@nodejs-github-bot nodejs-github-bot added test Issues and PRs related to the tests. v10.x labels Oct 12, 2018
@Trott Trott mentioned this pull request Oct 12, 2018
Closed
4 tasks
@richardlau
Copy link
Member

At first glance I don't think the test failure is related to this PR, perhaps some other v10.x backport?

https://ci.nodejs.org/job/node-test-commit-linuxone/nodes=rhel72-s390x/6096/console

18:49:40 not ok 1661 parallel/test-stream-wrap-drain
18:49:40   ---
18:49:40   duration_ms: 0.87
18:49:40   severity: fail
18:49:40   exitcode: 1
18:49:40   stack: |-
18:49:40     /data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-stream-wrap-drain.js:8
18:49:40     const { ShutdownWrap } = internalBinding('stream_wrap');
18:49:40                              ^
18:49:40     
18:49:40     TypeError: internalBinding is not a function
18:49:40         at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-stream-wrap-drain.js:8:26)
18:49:40         at Module._compile (internal/modules/cjs/loader.js:688:30)
18:49:40         at Object.Module._extensions..js (internal/modules/cjs/loader.js:699:10)
18:49:40         at Module.load (internal/modules/cjs/loader.js:598:32)
18:49:40         at tryModuleLoad (internal/modules/cjs/loader.js:537:12)
18:49:40         at Function.Module._load (internal/modules/cjs/loader.js:529:3)
18:49:40         at Function.Module.runMain (internal/modules/cjs/loader.js:741:12)
18:49:40         at startup (internal/bootstrap/node.js:285:19)
18:49:40         at bootstrapNodeJSCore (internal/bootstrap/node.js:739:3)
18:49:40   ...

@richardlau richardlau mentioned this pull request Oct 13, 2018
Closed
@richardlau
Copy link
Member

I raised #23633 to cover the failing test on v10.x-staging.

oyyd and others added 14 commits October 12, 2018 20:14
@oyyd @Trott
tls: make StreamWrap work correctly in "drain" callback
6d151ad 
When an instance of StreamWrap is shutting down and a "drain" event
is emitted, the instance will abort as its
`this[kCurrentShutdownRequest]` is already set. The following test
will fail before this commit.

PR-URL: nodejs#23294
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
@dyatlov @Trott
src: revert removal of SecureContext _external getter
f2b4734 
This `_external` getter is essential for some libs to work:
uWebSockets as an example.

PR-URL: nodejs#21711
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@sagitsofan @Trott
lib: http server, friendly error messages
ad59bbd 
Improved error message description for the http server binding errors.

Currently changed only in `setupListenHandle`, but needs to be
change all over.

Added new `uvExceptionWithHostPort` function (+export) in
`lib/internal/error.js` that extracts the error message defined by
libuv, using the error code, and returns an error object with the
full error description.

example:
old error message: `listen EADDRINUSE`
new error message: `listen EADDRINUSE: Address already in use`

Removed exportable function `_exceptionWithHostPort` from
`lib/util.js` - exported by accident

Replaced `exceptionWithHostPort` to the new function
`uvExceptionWithHostPort` for a more detailed error.

Fixes: nodejs#22936

PR-URL: nodejs#22995
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@gireeshpunathil @Trott
src: improve SSL version extraction logic
736d6d1 
The openssl version as defined in ssl libraries is complex.
The current logic to extract the major.minor.patch format
uses C semantics to loop through the text and search for
specific patterns. Use C++ string to tidy it up.

PR-URL: nodejs#23050
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@tniessen @Trott
src: fix bug in MallocedBuffer constructor
2218a38 
This should be a generic type even though we are
currently only using `char` as `T`.

PR-URL: nodejs#23434
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott
doc: revise security-reporting text in README
4b0c0f1 
Simplify and clarify the security-reporting text in the README. Now is
also probably a good time to ping the security triage folks to make sure
the text is still accurate.

PR-URL: nodejs#23407
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@joesepi @Trott
test: fixing assertion value order
f6a2af5 
PR-URL: nodejs#23574
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Hitesh Kanwathirtha <digitalinfinity@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@et4891 @Trott
test: fix assert.strictEqual argument order
4089a55 
PR-URL: nodejs#23518
Reviewed-By: Hitesh Kanwathirtha <digitalinfinity@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
@lundibundi @Trott
test: improve test-gc-http-client-connaborted
584470c 
* refactor out usage of 'function' for scoping
* wait till server is up to start firing requests

PR-URL: nodejs#23193
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@lundibundi @Trott
test: improve test-gc-http-client-onerror
0d7328d 
* refactor out usage of 'function' for scoping
* inline runTest function

PR-URL: nodejs#23196
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@addaleax @Trott
test: move some gc tests back to parallel/, unmark flaky
35f9218 
These should no longer be flaky after the libuv update.

Refs: nodejs#23336

PR-URL: nodejs#23356
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax @Trott
zlib: generate error code names in C++
e86b6aa 
This makes it easier to implement the lookup in a way that targets
error codes from a specific compression library, as a way towards
supporting multiple ones (e.g. brotli).

PR-URL: nodejs#23413
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@sam-github @Trott
doc: describe SNI host name format
3a533d6 
Ref: nodejs#23329

PR-URL: nodejs#23357
Refs: nodejs#23329
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@starkwang @Trott
http: reduce duplicated code for cleaning parser
7c9b37e 
PR-URL: nodejs#23351
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@Trott
Copy link
Member Author

Trott commented Oct 13, 2018

Now that #23633 has been fixed (we think), here's a CI again: https://ci.nodejs.org/job/node-test-pull-request/1​77​94

Trott added 2 commits October 12, 2018 20:18
@Trott
test: refactor common.ddCommand()
79a2b00 
* Remove different paths for Windows and POSIX.
* Remove fixtures file. Simply run the command immediately/directly.
* Since it is never called with more than one value for kilobytes,
  eliminate that argument.
* Update/simplify tests that use this function. (They no longer need to
  use child_process to run the command.)
* Update documentation.

PR-URL: nodejs#23411
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: George Adams <george.adams@uk.ibm.com>
@Trott
test: rename common.ddCommand()
ea510a6 
Change common.ddCommand() to common.createZeroFilledFile().

PR-URL: nodejs#23411
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: George Adams <george.adams@uk.ibm.com>
@Trott
Copy link
Member Author

Trott commented Oct 13, 2018 •
edited
Loading

Er... CI: https://ci.nodejs.org/job/node-test-pull-request/17807/

targos pushed a commit that referenced this pull request Oct 13, 2018
@Trott @targos
test: refactor common.ddCommand()
bb4da05 
* Remove different paths for Windows and POSIX.
* Remove fixtures file. Simply run the command immediately/directly.
* Since it is never called with more than one value for kilobytes,
  eliminate that argument.
* Update/simplify tests that use this function. (They no longer need to
  use child_process to run the command.)
* Update documentation.

Backport-PR-URL: #23630
PR-URL: #23411
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: George Adams <george.adams@uk.ibm.com>
targos pushed a commit that referenced this pull request Oct 13, 2018
@Trott @targos
test: rename common.ddCommand()
8def037 
Change common.ddCommand() to common.createZeroFilledFile().

Backport-PR-URL: #23630
PR-URL: #23411
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: George Adams <george.adams@uk.ibm.com>
@targos
Copy link
Member

targos commented Oct 13, 2018

Landed in bb4da05 and 8def037

@targos targos closed this Oct 13, 2018
MylesBorins pushed a commit that referenced this pull request Oct 30, 2018
@Trott @MylesBorins
test: refactor common.ddCommand()
2f84a18 
* Remove different paths for Windows and POSIX.
* Remove fixtures file. Simply run the command immediately/directly.
* Since it is never called with more than one value for kilobytes,
  eliminate that argument.
* Update/simplify tests that use this function. (They no longer need to
  use child_process to run the command.)
* Update documentation.

Backport-PR-URL: #23630
PR-URL: #23411
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: George Adams <george.adams@uk.ibm.com>
MylesBorins pushed a commit that referenced this pull request Oct 30, 2018
@Trott @MylesBorins
test: rename common.ddCommand()
2d32922 
Change common.ddCommand() to common.createZeroFilledFile().

Backport-PR-URL: #23630
PR-URL: #23411
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: George Adams <george.adams@uk.ibm.com>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
@Trott @rvagg
test: refactor common.ddCommand()
1a810b1 
* Remove different paths for Windows and POSIX.
* Remove fixtures file. Simply run the command immediately/directly.
* Since it is never called with more than one value for kilobytes,
  eliminate that argument.
* Update/simplify tests that use this function. (They no longer need to
  use child_process to run the command.)
* Update documentation.

Backport-PR-URL: #23630
PR-URL: #23411
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: George Adams <george.adams@uk.ibm.com>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
@Trott @rvagg
test: rename common.ddCommand()
43b426f 
Change common.ddCommand() to common.createZeroFilledFile().

Backport-PR-URL: #23630
PR-URL: #23411
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: George Adams <george.adams@uk.ibm.com>
MylesBorins pushed a commit that referenced this pull request Nov 29, 2018
@Trott @MylesBorins
test: refactor common.ddCommand()
1cdfd1a 
* Remove different paths for Windows and POSIX.
* Remove fixtures file. Simply run the command immediately/directly.
* Since it is never called with more than one value for kilobytes,
  eliminate that argument.
* Update/simplify tests that use this function. (They no longer need to
  use child_process to run the command.)
* Update documentation.

Backport-PR-URL: #23630
PR-URL: #23411
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: George Adams <george.adams@uk.ibm.com>
MylesBorins pushed a commit that referenced this pull request Nov 29, 2018
@Trott @MylesBorins
test: rename common.ddCommand()
9003c74 
Change common.ddCommand() to common.createZeroFilledFile().

Backport-PR-URL: #23630
PR-URL: #23411
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: George Adams <george.adams@uk.ibm.com>
@Trott Trott deleted the backport-23411 branch January 13, 2022 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau approved these changes

@addaleax addaleax addaleax approved these changes

@thefourtheye thefourtheye thefourtheye approved these changes

Assignees
No one assigned
Labels
test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.