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: add --repeat option to tools/test.py #7740

Closed
wants to merge 262 commits into from

Conversation

mhdawson
Copy link
Member

  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Description of change

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: #6700
Reviewed-By: Ben Noorhduis [email protected]
Reviewed-By: James M Snell [email protected]
Reviewed-By: Fedor Indutny [email protected]
Reviewed-By: thefourtheye - Sakthipriyan Vairamani [email protected]
Reviewed-By: joaocgreis - João Reis [email protected]
Reviewed-By: Johan Bergström [email protected]

trevnorris and others added 30 commits June 28, 2016 15:48
Make `HTTPParser` an instance of `AsyncWrap` and make it use
`MakeCallback`. This means that async wrap hooks will be called on
consumed TCP sockets as well as on non-consumed ones.

Additional uses of `AsyncCallbackScope` are necessary to prevent
improper state from progressing that triggers failure in the
test-http-pipeline-flood.js test. Optimally this wouldn't be necessary,
but for the time being it's the most sure way to allow operations to
proceed as they have.

Ref: nodejs#7048
Fix: nodejs#4416
PR-URL: nodejs#5419
Reviewed-By: Fedor Indutny <[email protected]>
In AsyncWrap::MakeCallback always return empty handle if there is an
error. In the future this should change to return a v8::MaybeLocal, but
that major change will have to wait for v6.x, and these changes are
meant to be backported to v4.x.

The HTTParser call to AsyncWrap::MakeCallback failed because it expected
a thrown call to return an empty handle.

In node::MakeCallback return an empty handle if the call is
in_makecallback(), otherwise return v8::Undefined() as usual to preserve
backwards compatibility.

Ref: nodejs#7048
Fixes: nodejs#5555
PR-URL: nodejs#5591
Reviewed-By: Julien Gilli <[email protected]>
Now that HTTPParser uses MakeCallback it is unnecessary to manually
process the nextTickQueue.

The KickNextTick function is now no longer needed so code has moved back
to node::MakeCallback to simplify implementation.

Include minor cleanup moving Environment::tick_info() call below the
early return to save an operation.

Ref: nodejs#7048
PR-URL: nodejs#5756
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Andreas Madsen <[email protected]>
Make comment clear that Undefined() is returned for legacy
compatibility. This will change in the future as a semver-major change,
but to be able to port this to previous releases it needs to stay as is.

Ref: nodejs#7048
PR-URL: nodejs#5756
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Andreas Madsen <[email protected]>
The number of callbacks accepted to setupHooks was getting unwieldy.
Instead change the implementation to accept an object with all callbacks

Ref: nodejs#7048
PR-URL: nodejs#5756
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Andreas Madsen <[email protected]>
The second argument of the post callback is a boolean indicating whether
the callback threw and was intercepted by uncaughtException or a domain.

Currently node::MakeCallback has no way of retrieving a uid for the
object. This is coming in a future patch.

Ref: nodejs#7048
PR-URL: nodejs#5756
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Andreas Madsen <[email protected]>
Rather than abort if the init/pre/post/final/destroy callbacks throw,
force the exception to propagate and not be made catchable. This way
the application is still not allowed to proceed but also allowed the
location of the failure to print before exiting. Though the stack itself
may not be of much use since all callbacks except init are called from
the bottom of the call stack.

    /tmp/async-test.js:14
      throw new Error('pre');
      ^
    Error: pre
        at InternalFieldObject.pre (/tmp/async-test.js:14:9)

Ref: nodejs#7048
PR-URL: nodejs#5756
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Andreas Madsen <[email protected]>
Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

Ref: nodejs#7048
PR-URL: nodejs#7096
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Andreas Madsen <[email protected]>
Respect the `{ family: 6 }` address family property when connecting to
a remote peer over TLS.

Fixes: nodejs#4139
Fixes: nodejs#6440
PR-URL: nodejs#6654
Reviewed-By: Colin Ihrig <[email protected]>
Update parallel/test-http-agent-getname to use assert.strictEqual()
consistently and const-ify variables while we're here.

PR-URL: nodejs#6654
Reviewed-By: Colin Ihrig <[email protected]>
Before this commit `node --debug-port=1234 debug t.js` ignored the
--debug-port= argument, binding to the default port 5858 instead,
making it impossible to debug more than one process on the same
machine that way.

This commit also reduces the number of places where the default port
is hard-coded by one.

Fixes: nodejs#3345
PR-URL: nodejs#3470
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: James M Snell <[email protected]>
On UNIX platforms, the debugger doesn't reliably kill the inferior when
killed by a signal.  Work around that by spawning the debugger in its
own process group and killing the process group instead of just the
debugger process.

This is a hack to get the continuous integration back to green, it
doesn't address the underlying issue, which is that the debugger
shouldn't leave stray processes behind.

Fixes: nodejs#7034
PR-URL: nodejs#7037
Refs: nodejs#3470
Reviewed-By: Colin Ihrig <[email protected]>
Changes to Node core in order to allow compilation for linuxOne.

The ../archs/linux32-s390x/opensslconf.h and
../archs/linux64-s390x/opensslconf.h were automatically
generated by running make linux-ppc linux-ppc64 in the
deps/openssl/config directory as per our standard
practice

After these changes we still need a version of v8
which supports linuxOne but that will be coming soon
in the 5.1 version of v8.  Until then with these changes
we'll be able to create a hybrid build which pulls in
v8 from the http://github/andrewlow repo.

PR-URL: nodejs#5941
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Ported by exinfinitum from a PR by jasnell:
see nodejs/node-v0.x-archive#14185

Allows the running of v8 tests on node's packaged v8 source code.

Note that the limited win32 support added by jasnell has NOT been ported,
and so these tests are currently UNIX ONLY.

Note that gclient depot tools
(see https://commondatastorage.googleapis.com/
chrome-infra-docs/flat/depot_tools/docs/html/
depot_tools_tutorial.html#_setting_up) and subversion are required
to run tests.

To perform tests, run the following commands:

make v8 DESTCPU=(ARCH)
make test-v8 DESTCPU=(ARCH)

where (ARCH) is your CPU architecture, e.g. x64, ia32.
DESTCPU MUST be specified for this to work properly.

Can also do tests on debug build by using "make test-v8 DESTCPU=(ARCH)
BUILDTYPE=Debug", or perform intl or benchmark tests via make
test-v8-intl or test-v8-benchmarks respectively.

Note that by default, quickcheck and TAP output are disabled, and i18n
is enabled. To activate these options, use options"QUICKCHECK=True" and
"ENABLE_V8_TAP=True" respectively.

Use "DISABLE_V8_I18N" to disable i18n.

Use V8_BUILD_OPTIONS to allow custom user-defined flags to be
appended onto "make v8".

Any tests performed after changes to the packaged v8 file will require
recompiling of v8, which can be done using "make v8 DESTCPU=(ARCH)".

Finally, two additional files necessary for one of the v8 tests have
been added to the v8 folder.

PR-URL: nodejs#4704
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: targos - Michaël Zasso <[email protected]>
Original commit message:

    [test] Set default locale in test runner

    BUG=v8:4437,v8:2899,chromium:604310
    LOG=n

    Review URL: https://codereview.chromium.org/1402373002

    Cr-Commit-Position: refs/heads/master@{nodejs#35614}

PR-URL: nodejs#7451
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Currently we do not specific an absolute path for the tap output of the
V8 test suite. This is proving to be unreliable across release lines.

By prepending `$(PWD)` to each path we can guarantee it will always be
in the root folder.

PR-URL: nodejs#7460
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
PR-URL: nodejs#6955
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Yuval Brik <[email protected]>
Make less assumptions about what objects will be available when
vm context creation or error message printing fail because V8
runs out of JS stack space.

Ref: nodejs#6899
PR-URL: nodejs#6907
Reviewed-By: Ben Noordhuis <[email protected]>
Don't cache the exported values of fully uninitialized builtins.
This works by adding an additional `loading` flag that is only
active during initial loading of an internal module and checking
that either the module is fully loaded or is in that state before
using its cached value.

This has the effect that builtins modules which could not be loaded
(e.g. because compilation failed due to missing stack space) can be
loaded at a later point.

Fixes: nodejs#6899

Ref: nodejs#6899
PR-URL: nodejs#6907
Reviewed-By: Ben Noordhuis <[email protected]>
This is part 1/2 of the fixes from v8:4871. This fixes a segfault in
verify-heap.

Original commit message:
  [crankshaft] Write fillers for folded old space allocations during verify-heap

  If we don't write fillers, we crash during PagedSpace verification when we try
  to iterate over dead memory (unused folded allocation slots).

  BUG=v8:4871,chromium:580959
  LOG=N

  Review URL: https://codereview.chromium.org/1837163002

  Cr-Commit-Position: refs/heads/master@{#35097}

Fixes: nodejs#5900
V8-Bug: https://bugs.chromium.org/p/v8/issues/detail?id=4871

PR-URL: nodejs#7303
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
This is part 2/2 of the fixes needed for v8:4871. This fix never landed
upstream because the bug is not present in active V8 version. The patch
is available from the upstream v8 bug however.

The segfault occurs at the intersection of the following three
conditions that are dependent on the allocation pattern of an
application: A pretenured (1) allocation site has to be optimized into
a merged allocation by the allocation folding optimization (2) and
there needs to be overflow of the store buffer (3).

This patch disables the allocation folding optimization for pretenured
allocations. This may have some, hopefully negligible, performance
impact on real world applications.

Fixes: nodejs#5900

PR-URL: nodejs#7303
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Currently, if an IPC event handler throws an error, it can
cause the message to not be consumed, leading to messages piling
up. This commit causes IPC events to be emitted on the next tick,
allowing the channel's processing logic to move forward as
normal.

Fixes: nodejs#6561
PR-URL: nodejs#6909
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
The test in this commit runs correctly if IPC messages are
properly consumed and emitted. Otherwise, the test times out.

Fixes: nodejs#6561
PR-URL: nodejs#6909
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
When a worker is disconnecting, it shuts down all of the handles
it is waiting on. It is possible that a handle does not have an
owner, which causes a crash. This commit closes such handles
without accessing the missing owner.

Fixes: nodejs#6561
PR-URL: nodejs#6909
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
By manually copying arguments and breaking the try/catch out, we are
able to improve the performance of util.format by 20-100% (depending on
the types).

PR-URL: nodejs#5360
Reviewed-By: James M Snell <[email protected]>
Replacing the regexp and replace function with a loop improves
performance by ~60-200%.

PR-URL: nodejs#5360
Reviewed-By: James M Snell <[email protected]>
This test checks that ownerless cluster worker handles are closed
correctly on disconnection.

Fixes: nodejs#6561
PR-URL: nodejs#6909
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Some CI jobs compile Node and run the tests on different machines.
This change enables collaborators to have finer control over what runs
on these jobs, such as the exact suites to run. The test-ci rule was
split into js and native, to allow for addons to be compiled only on
the machines that are going to run them.

Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
PR-URL: nodejs#7317
Adds `2` as a return value of `on_headers_complete`, this mode will be
used to fix handling responses to `CONNECT` requests.

See: nodejs#6198
PR-URL: nodejs#6279
Reviewed-By: Brian White <[email protected]>
Reviewed-By: James M Snell <[email protected]>
When handling a response to `CONNECT` request - skip message body
and do not attempt to parse the next message. `CONNECT` requests are
used in similar sense to HTTP Upgrade.

Fix: nodejs#6198
PR-URL: nodejs#6279
Reviewed-By: Brian White <[email protected]>
Reviewed-By: James M Snell <[email protected]>
bnoordhuis and others added 20 commits July 14, 2016 12:43
Use the overload of `v8::Function::NewInstance()` that returns a
`v8::MaybeLocal<v8::Object>`.  The overloads that return a simple
`v8::Local<v8::Object>` are deprecated.

PR-URL: nodejs#6652
Reviewed-By: Anna Henningsen <[email protected]>
* Make the 'extract embedded addons in the documentations' step a normal
  prerequisite.  As an order-only prerequisite, it's sometimes skipped
  when it shouldn't be.

* Make `tools/doc/addon-verify.js` a dependency of that step.  Changes
  to that file should result in a rebuild.

PR-URL: nodejs#6652
Reviewed-By: Anna Henningsen <[email protected]>
Introduced in commit 3f69ea5 ("tools: update marked dependency"), it
stopped the embedded addons in the documentation from getting built.

PR-URL: nodejs#6652
Reviewed-By: Ben Noordhuis <[email protected]>
Allows building just docs using existing Node instead of building Node
first.

PR-URL: nodejs#3888
Reviewed-By: Chris Dickinson <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
101dd1e introduced a regression in the doctool. This commit reverts
the changes that were made to the function signature of the various
doctool functions while maintaining support for passing in specific
node versions.

Refs: nodejs@101dd1e

PR-URL: nodejs#6680
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Robert Lindstaedt <[email protected]>
Adjust style in doctool tests to conform with predominant style of the
rest of the project. The biggest changes are:

* Replace string concatenation with `path.join()`
* Remove unnecessary quotes from property names

PR-URL: nodejs#6719
Reviewed-By: James M Snell <[email protected]>
These signatures were originally converted to opts hashes in nodejs#3888. That
change was misinterpreted as the intrinsic cause of a test failure and
reverted in nodejs#6680.

PR-URL: nodejs#6690
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Robert Jefe Lindstaedt <[email protected]>
Previously, output files which were created using includes (notably,
the single-page all.html) had basically broken internal links all
over the place because references like `errors.html#errors_class_error`
are being used, yet `id` attributes were generated that looked like
`all_class_error`.

This PR adds generation of comments from the include preprocessor
that indicate from which file the current markdown bits come and
lets the HTML output generation take advantage of that so that more
appropriate `id` attributes can be generated.

PR-URL: nodejs#6943
Reviewed-By: Robert Jefe Lindstaedt <[email protected]>
Reviewed-By: Daniel Wang <[email protected]>
Ref: nodejs#6578
PR-URL: nodejs#6864
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Claudio Rodriguez <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Add a `REPLACEME` tag that should be used when introducing
docs for new features, so that they can be updated when releases
are made.

Ref: nodejs#6578
PR-URL: nodejs#6864
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Claudio Rodriguez <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Skip the doctool tests when js-yaml, which is currently `require()`d
from the eslint source tree, is missing. This can happen, for example,
because eslint is not included in the release source tarballs.

Fixes: nodejs#7201
Ref: nodejs#6495
PR-URL: nodejs#7218
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
After the nodejs#3888 it was not possible to "make doc-only"
in some situations. This now fallsback to any installed
node version and throws "node not found" in error case.

Ref: nodejs#3888

PR-URL: nodejs#6906
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Add an option to the configure script for building d8.  Useful for
testing V8 standalone.

PR-URL: nodejs#7538
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
GCM cipher IV length can be >=1 bytes.
When not the default 12 bytes (96 bits) sets the IV length using
`EVP_CIPHER_CTX_ctrl` with type `EVP_CTRL_GCM_SET_IVLEN`

PR-URL: nodejs#6376
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
This commit removes some unnecessary signed checks on unsigned
variables.

PR-URL: nodejs#7174
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
This helps to prevent issues where a failed test can keep a bound
socket open long enough to cause other tests to fail with EADDRINUSE
because the same port number is used.

PR-URL: nodejs#7045
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Backported from
nodejs@99bf6fa

We can tell when `node-gyp` is changed by creating a prerequisite on
`deps/npm/node_modules/node-gyp/package.json`. The prerequisite is added
to the `test/addons/.buildstamp` since `build-addons` is .PHONY.

Testing for this change was entirely manual.

  $ make clean test-build # Initial build
  $ make test-build # Make sure build-addons doesn't rebuild
  $ touch deps/npm/node_modules/node-gyp/package.json # simulate change
  $ make test-build # Ensure build-addons rebuilds

PR-URL: nodejs#6787
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
This benchmark fails on Windows when trying to execute command which
is more than 32k in size. This commits skips this one case when running
under Windows.

PR-URL: nodejs#7178
Reviewed-By: Trott - Rich Trott <[email protected]>
Reviewed-By: orangemocha - Alexis Campailla <[email protected]>
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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: thefourtheye - Sakthipriyan Vairamani <[email protected]>
Reviewed-By: joaocgreis - João Reis <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
@mhdawson mhdawson self-assigned this Jul 14, 2016
@mhdawson mhdawson added the test Issues and PRs related to the tests. label Jul 14, 2016
@MylesBorins MylesBorins self-assigned this Jul 14, 2016
@mhdawson
Copy link
Member Author

@thealphanerd I think I you told me this landed. If so can it be closed ?

@MylesBorins
Copy link
Contributor

it is sitting on the head as 9308205

@mhdawson mhdawson deleted the repeatport2 branch March 15, 2017 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.