test: rename regression tests with descriptive file names (pt. 6) #19608
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TimothyGu
reviewed
Mar 27, 2018
Contributor
Author
There was a problem hiding this comment.
I think it's time I installed vscode-spell-check 😛
Member
There was a problem hiding this comment.
More helpful would be a comment that says what made this case special. (E.g., "This test ensures that Node.js deals with a large number of HTTP requests correctly." or some such.) Ditto with the test file name.
Member
Contributor
Author
|
@TimothyGu done. Please take a look, especially at the comment and let me know if it's satisfactory. |
TimothyGu
approved these changes
Mar 27, 2018
jasnell
approved these changes
Mar 28, 2018
Rename test-dgram-regress-4496 to test-dgram-typeerror-buffer to test-dgram-send-invalid-msg-type Rename test-http-regr-nodejsgh-2821 to test-http-request-large-payload Rename test-child-process-fork-regr-nodejsgh-2847 to test-child-process-fork-closed-channel-segfault Rename test-http-pipeline-regr-2639 to test-http-pipeline-serverresponse-assertionerror Rename test-http-pipeline-regr-3332 to test-http-pipeline-requests-connection-leak Rename test-http-pipeline-regr-3508 to test-http-pipeline-socket-parser-typeerror Refs: nodejs#19105 Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure
ryzokuken
force-pushed
the
test/regress-1
branch
from
122fe0e to
73b4dee
Compare
Member
trivikr
approved these changes
Mar 28, 2018
Member
|
Landed in ff7c2cc |
trivikr
pushed a commit
that referenced
this pull request
Mar 28, 2018
Refs: #19105 Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure PR-URL: #19608 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos
pushed a commit
that referenced
this pull request
Apr 2, 2018
Refs: #19105 Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure PR-URL: #19608 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rename the tests appropriately alongside mentioning the subsystem
Also, make a few basic changes to make sure the tests conform to the standard test structure
Refs: #19105
Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure
Checklist
P.S. Two commits (f84a6b7 and 2ee8d87) are unrelated and cancel each other out. They won't matter after we squash.