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

test: fix test-cluster-dgram-1 flakiness - 4.x backport #9109

Closed
wants to merge 204 commits into from

Conversation

santigimeno
Copy link
Member

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change

Check for the number of messages received in the exit event listener
instead of the disconnect listener.

Fixes: #8380
Ref: #8383
Reviewed-By: Colin Ihrig cjihrig@gmail.com
Reviewed-By: Michael Dawson michael_dawson@ca.ibm.com
Reviewed By: James M Snell jasnell@gmail.com

bnoordhuis and others added 30 commits September 28, 2016 10:37
Add an option to the configure script for building d8.  Useful for
testing V8 standalone.

PR-URL: nodejs#7538
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Manual backport from master to 4.x stream, original
commit message follows.

I often want to run a test many times to see if a failure
can be recreated and I believe this is a common
use case.  We even have this job in the CI
https://ci.nodejs.org/job/node-stress-single-test/configure
but often you want to run it on a specific machine.

This patch adds the --repeat option so that
you can repeat the selected set of tests a
number of times. Given existing options
in test.py this will allow you to run
one or more tests for the number of
repeats specified. For example:

tools/test.py -j8 --repeat 1000 parallel/test-process-exec-argv

runs the test-process-exec-argv test 1000 times,
running 8 copies in parallel

tools/test.py --repeat 2

would run the entire test suite twice.

PR-URL: nodejs#6700
Reviewed-By: Ben Noorhduis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: thefourtheye - Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: joaocgreis - João Reis <reis@janeasystems.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Enable targetting of a different node version than
the currently running one when building binary modules.

Based on nodejs/node@410296c37

PR-URL: nodejs#8171
Ref: nodejs#8027
Ref: nodejs#7808
Ref: nodejs/node-gyp#855
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Minor rewording related to making a server listen to a random port,
and added how to retrieve which port was randomly chosen by the OS.

Also changed documented `server.listen()` signature as it does in fact
not require `port` to be provided.

PR-URL: nodejs#8025
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Exclude tests for AIX in 4.x stream now that
its been added to regular regression runs.  This
will avoid known failures from making the build
look RED while also being able to catch any
new regressions if they are introduced.

PR-URL: nodejs#8076
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joao Reis <reis@janeasystems.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#7479
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The function objects encapsulating `isIPv4` and `isIPv6` are not
necessary. They can be directly exposed from `cares`.

PR-URL: nodejs#7481
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Inspect boxed symbol objects in the same way other boxed primitives
are inspected.

Fixes: nodejs#7639
PR-URL: nodejs#7641
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Use `msvs_settings.MASM.UseSafeExceptionHandlers` when building OpenSSL
assembly code on Windows. This option appends `/safeseh` to the list of
assembler flags when building `.asm` files on Windows.

Having this option in place, separate rules in `masm_compile.gypi` are
no longer needed.

Fix: nodejs#7426
PR-URL: nodejs#7427
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bert Belder <bertbelder@gmail.com>
`ml64.exe` doesn't support `/safeseh` option. Do not attempt to use it
if `target_arch=="x64"`.

See: https://msdn.microsoft.com/en-us/library/s0ksfwcf.aspx
PR-URL: nodejs#7759
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Since I was doing the necessary git archaeology anyway, I took the time
to add YAML information to the docs about when `addMembership()` and
`dropMembership()` first appeared in their current forms.

PR-URL: nodejs#6753
Ref: nodejs#6578
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This change is in preparation for lint-enforced brace style.

PR-URL: nodejs#8348
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
This change is in preparation for lint enforcement of brace style.

PR-URL: nodejs#8348
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Enable `brace-style` in ESLint.

Ref: nodejs#7094 (comment)

PR-URL: nodejs#8348
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Set the `req.buffer` property, which serves as a way of keeping
a `Buffer` alive that is being written to a stream, on the C++
side instead of the JS side.

This closes a hole where buffers that were temporarily created
in order to write strings with uncommon encodings (e.g. `hex`)
were passed to the native side without being set as `req.buffer`.

Fixes: nodejs#8251
PR-URL: nodejs#8252
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
We will be introducing many more critical sections in the upcoming
multi-isolate changes, so let's make manual synchronization a thing
of the past.

PR-URL: nodejs#7334
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Extend linting to tools/license2rtf.js and any other JS that gets added
to the `tools` directory by default.

This incidentally simplifies lint invocation.

Ref: nodejs#8349
PR-URL: nodejs#7647
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
`POST_STATUS_TO_PR` previously did not work.

Now it works.

Update the onboarding documentation accordingly.

PR-URL: nodejs#8059
Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#8060
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Some constants in the zlib docs are not in the actual code:

zlib.Z_BINARY
zlib.Z_TEXT
zlib.Z_ASCII
zlib.Z_UNKNOWN

Also handled in nodejs#7203, but marked as
semver-major, so will not land in v6.x.

Fixes: nodejs#7204
PR-URL: nodejs#7520
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
The arguments object is not created for arrow functions so the example
was incorrect.

PR-URL: nodejs#7674
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Update some outdated material. Provide some minor fixes. Wrap to 80
characters.

PR-URL: nodejs#7719
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
PR-URL: nodejs#7801
There has been some question about whether the #node.js irc channel
falls under the TSC oversight or not.

See: nodejs#7746

This clarifies that the #node.js irc channel is a community provided
resource that is not currently directly under the oversight of the
TSC/CTC.

PR-URL: nodejs#7810
Reviewed-By: Bryan Hughes <bryan@nebri.us>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#7877
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
- Specify that the ‘make test’ commands are Unix/OS X specific.
- Link to BUILDING.md for other platform commands.

Fixes: nodejs#7646
PR-URL: nodejs#7783
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
CTC quorum rules were not in writing. There was an informal
understanding between CTC members. Document the rules to avoid
differences in interpretation.

PR-URL: nodejs#7813
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Provide example activities to better distinguish Collaborator activities
from CTC member activities.

PR-URL: nodejs#7744
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
PR-URL: nodejs#7900
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Add language identifying a request for voluntary resignation as the
typical mechanism for addressing inactive CTC members.

PR-URL: nodejs#7720
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Trott and others added 17 commits October 10, 2016 19:11
PR-URL: nodejs#8176
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
There is no test coverage for `assert.notDeepStrictEqual()`. Add some
minimal tests.

PR-URL: nodejs#8177
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
The test does some extra work that isn't necessary because of the way
temp directories are handled. The test removes all files from the temp
directory with `common.refreshTmpDir()` but still filters the results
even though only its files will be in the directory).

Refactor to remove that unneeded logic.

PR-URL: nodejs#8180
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Previously, an attempt was made to make sure the links state is
inherited. Unfortunately, this support was not complete, which
results in various unresolved links in the JSON output (as an
example, [1] contains `initialized by calling
[<code>buf.fill(fill, encoding)</code>][<code>buf.fill()</code>]`).

This commit completes that attempt. After this commit, individual
instances of the parser (for descriptions) inherit the links state
from the root lexer, so that individual Markdown links in descriptions
could be resolved. That same example is now substituted with
`initialized by calling <a href=\"#buffer_buf_fill_value_offset_end_encoding\"><code>buf.fill(fill, encoding)</code></a>`.

[1]: https://nodejs.org/api/buffer.json

PR-URL: nodejs#8494
Reviewed-By: Roman Reiss <me@silverwind.io>
PR-URL: nodejs#7915
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#8365
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
PR-URL: nodejs#8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: nodejs#8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: nodejs#8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The RELEASE_URLBASE environment variable is used in releases as
a prefix for links in the process.release object. The Makefile picks
it and forwards it to configure, but vcbuild.bat did not. Hence, in
Windows, Node releases have a correct process.release because it uses
the default URL, but nightlies, RCs and so on do not, breaking
node-gyp. This enables native modules to be built with such versions
of Node.

PR-URL: nodejs#8430
Reviewed-By: jbergstroem - Johan Bergström <bugs@bergstroem.nu>
`convertNPNProtocols` and `convertALPNProtocols' uses the `protocols`
buffer object as it is, and if it is modified outside of core, it
might have an impact. This patch makes a copy of the buffer object,
before using it.

PR-URL: nodejs#8055
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Fix handle leaks in Buffer::New() and Buffer::Copy() by creating the
handle scope before looking up the env with Environment::GetCurrent().

Environment::GetCurrent() calls v8::Isolate::GetCurrentContext(), which
creates a handle in the current scope, i.e., the scope created by the
caller of Buffer::New() or Buffer::Copy().

PR-URL: nodejs#7711
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Create a handle scope before performing a check that creates a handle,
otherwise the handle is leaked into the handle scope of the caller.

PR-URL: nodejs#7711
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Create a handle scope before performing a check that creates a handle,
otherwise the handle is leaked into the handle scope of the caller.

PR-URL: nodejs#7711
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
API function callbacks run inside an implicit HandleScope.  We don't
need to explicitly create one and in fact introduce some unnecessary
overhead when we do.

PR-URL: nodejs#7711
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Incorrect string length calculation when passing escaped dot.

- CVE: CVE-2016-5180
- Upstream bug: https://c-ares.haxx.se/adv_20160929.html

Ref: nodejs#9037
PR-URL: nodejs#8849
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Run `npm install` before building the documentation from release
tarballs. The doctool currently depends on `js-yaml`, which
is imported from the `tools/eslint` subtree; however, release
tarballs don’t contain that directory.

Running `npm install` is clearly not a beautiful solution,
but it works.

Fixes: nodejs#7872
PR-URL: nodejs#8413
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
@nodejs-github-bot nodejs-github-bot added test Issues and PRs related to the tests. v4.x labels Oct 15, 2016
Check for the number of messages received in the `exit` event listener
instead of the `disconnect` listener.

Fixes: nodejs#8380
Ref: nodejs#8383
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed By: James M Snell <jasnell@gmail.com>
@mscdex mscdex added cluster Issues and PRs related to the cluster subsystem. dgram Issues and PRs related to the dgram subsystem / UDP. labels Oct 15, 2016
Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM

@MylesBorins MylesBorins added this to the v4.6.2 milestone Oct 24, 2016
@MylesBorins
Copy link
Contributor

landed in ecf8f5a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cluster Issues and PRs related to the cluster subsystem. dgram Issues and PRs related to the dgram subsystem / UDP. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.