-
- Notifications
You must be signed in to change notification settings - Fork 33.8k
test: refactor test-stream2-writable #10353
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a slight difference between setTimeout and setImmediate, given where they are executed in the event loop. If it is indifferent for the test, e.g. even process.nextTick is fine, then add it as a comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverting to setTimeout() just to be 100% sure we're not modifying the test behavior in a way that might undermine the purpose of the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do process.nextTick(process.nextTick(setImmediate( you should virtually guarantee adding it to the end of the next processing of immediates, fwiw.
mcollina left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a nit
* replace setTimeout() with setImmediate() * assert.equal() -> assert.strictEqual() * remove unused function arguments * normalize indentation
0135334 to 6805678 Compare * add duration to setTimeout() * assert.equal() -> assert.strictEqual() * remove unused function arguments * normalize indentation PR-URL: nodejs#10353 Reviewed-By: Julian Duque <julianduquej@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
| Landed in 7472521. |
* add duration to setTimeout() * assert.equal() -> assert.strictEqual() * remove unused function arguments * normalize indentation PR-URL: #10353 Reviewed-By: Julian Duque <julianduquej@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
* add duration to setTimeout() * assert.equal() -> assert.strictEqual() * remove unused function arguments * normalize indentation PR-URL: #10353 Reviewed-By: Julian Duque <julianduquej@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
* add duration to setTimeout() * assert.equal() -> assert.strictEqual() * remove unused function arguments * normalize indentation PR-URL: #10353 Reviewed-By: Julian Duque <julianduquej@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
* add duration to setTimeout() * assert.equal() -> assert.strictEqual() * remove unused function arguments * normalize indentation PR-URL: #10353 Reviewed-By: Julian Duque <julianduquej@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
* add duration to setTimeout() * assert.equal() -> assert.strictEqual() * remove unused function arguments * normalize indentation PR-URL: #10353 Reviewed-By: Julian Duque <julianduquej@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
* add duration to setTimeout() * assert.equal() -> assert.strictEqual() * remove unused function arguments * normalize indentation PR-URL: #10353 Reviewed-By: Julian Duque <julianduquej@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
* add duration to setTimeout() * assert.equal() -> assert.strictEqual() * remove unused function arguments * normalize indentation PR-URL: #10353 Reviewed-By: Julian Duque <julianduquej@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
* add duration to setTimeout() * assert.equal() -> assert.strictEqual() * remove unused function arguments * normalize indentation PR-URL: #10353 Reviewed-By: Julian Duque <julianduquej@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test stream
Description of change