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

Backport 2253 #8340

Closed
wants to merge 7 commits into from
Closed

Backport 2253 #8340

wants to merge 7 commits into from

Conversation

bmeck
Copy link
Member

@bmeck bmeck commented Aug 30, 2016

Checklist
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

tests

Description of change

add tests for the behavior of using -r and --require to ensure they are preloaded even when using interactive/streamed entry points

bnoordhuis and others added 7 commits August 16, 2016 12:04
@bnoordhuis
build: add --enable-d8 configure option
cd55b79 
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>
@mhdawson
test: add --repeat option to tools/test.py
9308205 
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>
@saper @jasnell
build: add node_module_version to config.gypi
a0a1de1 
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>
@phillipj @jasnell
doc: improve server.listen() random port
3ecd06d 
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>
@mhdawson
test: exclude tests for AIX
4bfb247 
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>
@bmeck
test: test preloaded modules when using stdin or repl
818f46d 
This test fails on Solaris, see nodejs#2253
@Trott @bmeck
test: fix flaky test-preload
b98656e 
Use `close` event rather than `exit` event to make sure all output has
been received before checking assertions.

Fixes: nodejs#6722
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Aug 30, 2016
@bmeck bmeck mentioned this pull request Aug 30, 2016
Closed
@mscdex mscdex added v4.x module Issues and PRs related to the module subsystem. labels Aug 30, 2016
@MylesBorins
Copy link
Contributor

hey @bmeck is looks like this is failing

Path: parallel/test-preload
assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: 'hello\n' == 'A\nhello\n'
    at ChildProcess.<anonymous> (/Users/thealphanerd/code/node/test/parallel/test-preload.js:84:10)
    at emitTwo (events.js:87:13)
    at ChildProcess.emit (events.js:172:7)
    at maybeClose (internal/child_process.js:829:16)
    at Socket.<anonymous> (internal/child_process.js:319:11)
    at emitOne (events.js:77:13)
    at Socket.emit (events.js:169:7)
    at Pipe._onclose (net.js:486:12)

@bmeck
Copy link
Member Author

bmeck commented Sep 4, 2016

As stated yes. This just adds tests

On Sep 4, 2016 1:38 PM, "Myles Borins" notifications@github.com wrote:

hey @bmeck https://github.com/bmeck is looks like this is failing

Path: parallel/test-preload
assert.js:89
throw new assert.AssertionError({
^
AssertionError: 'hello\n' == 'A\nhello\n'
at ChildProcess. (/Users/thealphanerd/code/node/test/parallel/test-preload.js:84:10)
at emitTwo (events.js:87:13)
at ChildProcess.emit (events.js:172:7)
at maybeClose (internal/child_process.js:829:16)
at Socket. (internal/child_process.js:319:11)
at emitOne (events.js:77:13)
at Socket.emit (events.js:169:7)
at Pipe._onclose (net.js:486:12)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#8340 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAOUo7SKWcjKl4QUSbA3ZmHUdQE0vV6dks5qmxA9gaJpZM4JwmAv
.

@MylesBorins
Copy link
Contributor

@bmeck I am a bit confused then. Adding this should not break the local test suite. Am I missing something?

@bmeck
Copy link
Member Author

bmeck commented Sep 7, 2016 •
edited
Loading

@thealphanerd this only adds tests, v4 does not have correct behavior. original issue that led to these tests could never deduce the cause of the race/error. Tests were rebased onto newer branch, issue was gone. Guessing w/e fixed v6 was not backported to v4

@MylesBorins
Copy link
Contributor

@bmeck thanks! I'll keep this one in the background then and revisit after the next round of audits to see if it is fixed

@MylesBorins MylesBorins force-pushed the v4.x-staging branch 2 times, most recently from 6a126fb to 004d665 Compare October 10, 2016 22:18
@MylesBorins
Copy link
Contributor

I'm closing this for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@MylesBorins MylesBorins

Labels
module Issues and PRs related to the module subsystem. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@bmeck @MylesBorins @mscdex @nodejs-github-bot @bnoordhuis @mhdawson @saper @phillipj @Trott