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

test: move ArrayStream to common #4027

Merged
merged 0 commits into from
Nov 27, 2015
Merged

test: move ArrayStream to common #4027

merged 0 commits into from
Nov 27, 2015

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Nov 25, 2015

A number of REPL tests define the same ArrayStream object. This commit moves the repeated code into common.js.

@bnoordhuis
Copy link
Member

LGTM. test/common.js is arguably a mildly odd place for it but putting it in e.g. test/fixtures is weird too.

You could drop it in test/parallel and call it array-stream.js. The test runner will skip over it, it only executes files that start with test-. No strong opinion though, just thinking out loud.

@cjihrig
Copy link
Contributor Author

cjihrig commented Nov 25, 2015

I agree that there isn't a great location for it. My only concern with putting it in test/parallel is that eventually it might be needed in sequential or somewhere else. That's not really a big deal either though.

@bnoordhuis
Copy link
Member

I guess it doesn't really matter, we can always move it later. LGTM.

@mscdex mscdex added repl Issues and PRs related to the REPL subsystem. test Issues and PRs related to the tests. labels Nov 25, 2015
@cjihrig
Copy link
Contributor Author

cjihrig commented Nov 26, 2015

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

@cjihrig cjihrig closed this Nov 27, 2015
cjihrig added a commit to cjihrig/node that referenced this pull request Nov 27, 2015
@cjihrig
test: move ArrayStream to common
f2e319b 
A number of REPL tests define the same ArrayStream object. This
commit moves the repeated code into common.js.

PR-URL: nodejs#4027
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@cjihrig cjihrig merged commit f2e319b into nodejs:master Nov 27, 2015
@cjihrig cjihrig deleted the arraystream branch November 27, 2015 02:22
@MylesBorins
Copy link
Contributor

For this to land on LTS we need #4013 to land first, which is blocked by #4026

@cjihrig
Copy link
Contributor Author

cjihrig commented Dec 1, 2015

This could be ported without the other two PRs if you really had your heart set on it. The diff should only be one test introduced in #4013. That said, it's not important.

cjihrig added a commit that referenced this pull request Dec 5, 2015
@cjihrig @rvagg
test: move ArrayStream to common
69c95bb 
A number of REPL tests define the same ArrayStream object. This
commit moves the repeated code into common.js.

PR-URL: #4027
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This was referenced Dec 8, 2015
Closed
Merged
@rvagg rvagg mentioned this pull request Dec 17, 2015
Closed
@MylesBorins MylesBorins mentioned this pull request Dec 30, 2015
Closed
cjihrig added a commit to cjihrig/node that referenced this pull request Jan 7, 2016
@cjihrig
test: move ArrayStream to common
f64aa34 
A number of REPL tests define the same ArrayStream object. This
commit moves the repeated code into common.js.

PR-URL: nodejs#4027
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Conflicts:
	test/common.js
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
@cjihrig
test: move ArrayStream to common
acc3d66 
A number of REPL tests define the same ArrayStream object. This
commit moves the repeated code into common.js.

PR-URL: #4027
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Conflicts:
	test/common.js
@MylesBorins MylesBorins mentioned this pull request Jan 19, 2016
Merged
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
@cjihrig
test: move ArrayStream to common
398f979 
A number of REPL tests define the same ArrayStream object. This
commit moves the repeated code into common.js.

PR-URL: nodejs#4027
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
jaimecbernardo added a commit to JaneaSystems/node that referenced this pull request Jun 30, 2017
@jaimecbernardo
test,fs: delay unlink in test-regress-nodejsGH-4027.js
e0dfdd2 
The sequential/test-regress-GH-4027 test is flaky with an increased
system load, failing when the watched file is unlinked before the
first state of the watched file is retrieved.

After increasing the delay before unlinking and calling setTimeout
after watchFile, the flakiness stopped reproducing.

Fixes: nodejs#13800
Trott pushed a commit that referenced this pull request Jul 3, 2017
@jaimecbernardo @Trott
test,fs: delay unlink in test-regress-GH-4027.js
f1c890a 
The sequential/test-regress-GH-4027 test is flaky with an increased
system load, failing when the watched file is unlinked before the
first state of the watched file is retrieved.

After increasing the delay before unlinking and calling setTimeout
after watchFile, the flakiness stopped reproducing.

PR-URL: #14010
Fixes: #13800
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
addaleax pushed a commit that referenced this pull request Jul 11, 2017
@jaimecbernardo @addaleax
test,fs: delay unlink in test-regress-GH-4027.js
2daff32 
The sequential/test-regress-GH-4027 test is flaky with an increased
system load, failing when the watched file is unlinked before the
first state of the watched file is retrieved.

After increasing the delay before unlinking and calling setTimeout
after watchFile, the flakiness stopped reproducing.

PR-URL: #14010
Fixes: #13800
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@addaleax addaleax mentioned this pull request Jul 11, 2017
Merged
addaleax pushed a commit that referenced this pull request Jul 18, 2017
@jaimecbernardo @addaleax
test,fs: delay unlink in test-regress-GH-4027.js
b9e07f9 
The sequential/test-regress-GH-4027 test is flaky with an increased
system load, failing when the watched file is unlinked before the
first state of the watched file is retrieved.

After increasing the delay before unlinking and calling setTimeout
after watchFile, the flakiness stopped reproducing.

PR-URL: #14010
Fixes: #13800
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Jul 19, 2017
@jaimecbernardo @Fishrock123
test,fs: delay unlink in test-regress-GH-4027.js
95ed925 
The sequential/test-regress-GH-4027 test is flaky with an increased
system load, failing when the watched file is unlinked before the
first state of the watched file is retrieved.

After increasing the delay before unlinking and calling setTimeout
after watchFile, the flakiness stopped reproducing.

PR-URL: #14010
Fixes: #13800
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit that referenced this pull request Aug 15, 2017
@jaimecbernardo @MylesBorins
test,fs: delay unlink in test-regress-GH-4027.js
03a5c5a 
The sequential/test-regress-GH-4027 test is flaky with an increased
system load, failing when the watched file is unlinked before the
first state of the watched file is retrieved.

After increasing the delay before unlinking and calling setTimeout
after watchFile, the flakiness stopped reproducing.

PR-URL: #14010
Fixes: #13800
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit that referenced this pull request Aug 15, 2017
@jaimecbernardo @MylesBorins
test,fs: delay unlink in test-regress-GH-4027.js
f9a472f 
The sequential/test-regress-GH-4027 test is flaky with an increased
system load, failing when the watched file is unlinked before the
first state of the watched file is retrieved.

After increasing the delay before unlinking and calling setTimeout
after watchFile, the flakiness stopped reproducing.

PR-URL: #14010
Fixes: #13800
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
@jaimecbernardo @MylesBorins
test,fs: delay unlink in test-regress-GH-4027.js
f3c0b8c 
The sequential/test-regress-GH-4027 test is flaky with an increased
system load, failing when the watched file is unlinked before the
first state of the watched file is retrieved.

After increasing the delay before unlinking and calling setTimeout
after watchFile, the flakiness stopped reproducing.

PR-URL: #14010
Fixes: #13800
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Aug 16, 2017
Merged
This was referenced Sep 5, 2017
Closed
Closed
Closed
Closed
Closed
@ryzokuken ryzokuken mentioned this pull request Mar 6, 2018
Closed
8 tasks
ryzokuken added a commit to ryzokuken/node that referenced this pull request Mar 6, 2018
@ryzokuken
test: rename test-regress-nodejsGH-4027
7b91ada 
Rename the test appropriately alongside mentioning the subsystem
Also, make a few basic changes to make sure the test conforms
to the standard test structure

Refs: nodejs#19105
Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure
devsnek pushed a commit that referenced this pull request Mar 8, 2018
@ryzokuken @devsnek
test: rename test-regress-GH-4027
d0518c2 
Rename the test appropriately alongside mentioning the subsystem
Also, make a few basic changes to make sure the test conforms
to the standard test structure

Refs: #19105
Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure

PR-URL: #19161
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
targos pushed a commit that referenced this pull request Mar 17, 2018
@ryzokuken @targos
test: rename test-regress-GH-4027
21b3f3f 
Rename the test appropriately alongside mentioning the subsystem
Also, make a few basic changes to make sure the test conforms
to the standard test structure

Refs: #19105
Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure

PR-URL: #19161
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
@targos targos mentioned this pull request Mar 18, 2018
Merged
MylesBorins pushed a commit that referenced this pull request Mar 20, 2018
@ryzokuken @MylesBorins
test: rename test-regress-GH-4027
2262a34 
Rename the test appropriately alongside mentioning the subsystem
Also, make a few basic changes to make sure the test conforms
to the standard test structure

Refs: #19105
Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure

PR-URL: #19161
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
@ryzokuken @MayaLekova
test: rename test-regress-nodejsGH-4027
ebdb13a 
Rename the test appropriately alongside mentioning the subsystem
Also, make a few basic changes to make sure the test conforms
to the standard test structure

Refs: nodejs#19105
Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure

PR-URL: nodejs#19161
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
trivikr pushed a commit to trivikr/node that referenced this pull request Sep 15, 2018
@ryzokuken @MylesBorins
test: rename test-regress-nodejsGH-4027
b70dd35 
Rename the test appropriately alongside mentioning the subsystem
Also, make a few basic changes to make sure the test conforms
to the standard test structure

Refs: nodejs#19105
Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure

PR-URL: nodejs#19161
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
repl Issues and PRs related to the REPL 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.
5 participants
@cjihrig @bnoordhuis @MylesBorins @mscdex @jasnell