FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: rename regression tests with descriptive file names (pt. 4) by ryzokuken · Pull Request #19332 · nodejs/node · GitHub

/ node Public

test: rename regression tests with descriptive file names (pt. 4) - #19332

Closed
ryzokuken wants to merge 1 commit into
nodejs:masterfrom
ryzokuken:test/regress
ConversationCommits (1)ChecksFiles changed
Closed

test: rename regression tests with descriptive file names (pt. 4)#19332
ryzokuken wants to merge 1 commit into
nodejs:masterfrom
ryzokuken:test/regress

Conversation

ryzokuken commented Mar 13, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

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

Checklist

  • test/parallel/test-regress-GH-io-1068.js
  • test/parallel/test-regress-GH-io-1811.js
  • test/parallel/test-regress-GH-node-9326.js

Additional files renamed

  • timers-regress-GH-9765
  • test-tls-pfx-gh-5100-regr
  • test-tls-regr-gh-5108

nodejs-github-bot added the test Issues and PRs related to the tests. label Mar 13, 2018

ryzokuken commented Mar 13, 2018

Copy link
Copy Markdown
Contributor Author

PSA: Naming commits poorly on purpose so that it's easier to rebase it in the end.

joyeecheung commented Mar 14, 2018

Copy link
Copy Markdown
Member

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

lpinca added the wip Issues and PRs that are still a work in progress. label Mar 14, 2018

lpinca commented Mar 14, 2018

Copy link
Copy Markdown
Member

@ryzokuken added "in progress" label as I assumed you want to rename more tests from the checklist in the description. Let me know If I'm wrong.

ryzokuken commented Mar 15, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@lpinca you're quite right, thanks. Sorry for not being able to reply immediately.

ryzokuken commented Mar 15, 2018

Copy link
Copy Markdown
Contributor Author

@joyeecheung the original 3 have been fixed and that ends the list in #19105. Thus, I'm adding the "Fixes: ..." reference, let me know if I shouldn't.

lpinca removed the wip Issues and PRs that are still a work in progress. label Mar 16, 2018
lpinca approved these changes Mar 16, 2018
View reviewed changes
Comment thread test/parallel/test-kill-segfault-freebsd.js Outdated

lpinca Mar 16, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Nit: can you please use "Node.js"?

Comment thread test/parallel/test-tls-tlswrap-segfault.js Outdated

lpinca Mar 16, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ditto.

Comment thread test/parallel/test-tty-stdin-end.js Outdated

lpinca Mar 16, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Same thing.

ryzokuken commented Mar 16, 2018

Copy link
Copy Markdown
Contributor Author

@lpinca done.

lpinca commented Mar 16, 2018

Copy link
Copy Markdown
Member

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

lpinca added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Mar 16, 2018
lpinca reviewed Mar 16, 2018
View reviewed changes
Comment thread test/parallel/test-zlib-kmaxlength-rangeerror.js Outdated

lpinca Mar 16, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@ryzokuken it seems that the order of these requires was part of the test. This test is now failing on all machines. Please revert this particular change.

ryzokuken Mar 17, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

My bad. Fixing this in a sec.

lpinca removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Mar 16, 2018

ryzokuken commented Mar 17, 2018

Copy link
Copy Markdown
Contributor Author

@lpinca done. I shouldn't have meddled with it in the first place.

lpinca commented Mar 17, 2018

Copy link
Copy Markdown
Member

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

ryzokuken commented Mar 17, 2018

Copy link
Copy Markdown
Contributor Author

@lpinca they passed! 🎉

lpinca added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Mar 18, 2018

lpinca commented Mar 18, 2018

Copy link
Copy Markdown
Member

@ryzokuken would you mind squashing commits? Thank you.

ryzokuken commented Mar 18, 2018

Copy link
Copy Markdown
Contributor Author

@lpinca sure! Just give me a second.

test: rename regression tests with descriptive file names
1db4202 
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

1. Renamed test-regress-GH-io-1068 to test-tty-stdin-end
2. Renamed test-regress-GH-io-1811 to test-zlib-kmaxlength-rangeerror
3. Renamed test-regress-GH-node-9326 to test-kill-segfault-freebsd
4. Renamed test-timers-regress-nodejsGH-9765 to test-timers-setimmediate-infinite-loop
5. Renamed test-tls-pfx-nodejsgh-5100-regr to test-tls-pfx-authorizationerror
6. Renamed test-tls-regr-nodejsgh-5108 to test-tls-tlswrap-segfault

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

ryzokuken commented Mar 18, 2018

Copy link
Copy Markdown
Contributor Author

@lpinca looks good enough?

lpinca commented Mar 18, 2018

Copy link
Copy Markdown
Member

Yes, perfect.
CI: https://ci.nodejs.org/job/node-test-pull-request/13730/

ryzokuken commented Mar 18, 2018

Copy link
Copy Markdown
Contributor Author

@lpinca any idea what could've gone wrong?

lpinca commented Mar 18, 2018

Copy link
Copy Markdown
Member

AIX failure is not related, will land this later.

lpinca commented Mar 18, 2018

Copy link
Copy Markdown
Member

Landed in d54e0f8.

lpinca pushed a commit that referenced this pull request Mar 18, 2018
test: rename regression tests file names
d54e0f8 
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.

- Rename test-regress-GH-io-1068 to test-tty-stdin-end
- Rename test-regress-GH-io-1811 to test-zlib-kmaxlength-rangeerror
- Rename test-regress-GH-node-9326 to test-kill-segfault-freebsd
- Rename test-timers-regress-GH-9765 to test-timers-setimmediate-infinite-loop
- Rename test-tls-pfx-gh-5100-regr to test-tls-pfx-authorizationerror
- Rename test-tls-regr-gh-5108 to test-tls-tlswrap-segfault

PR-URL: #19332
Fixes: #19105
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
lpinca closed this Mar 18, 2018

lpinca commented Mar 18, 2018

Copy link
Copy Markdown
Member

There are a few more that weren't listed in the tracking issue:

test/parallel/test-arm-math-exp-regress-1376.js
test/parallel/test-buffer-regression-649.js
test/parallel/test-dgram-regress-4496.js
test/parallel/test-dns-regress-7070.js
test/parallel/test-http-agent-maxsockets-regress-4050.js

ryzokuken commented Mar 18, 2018

Copy link
Copy Markdown
Contributor Author

@lpinca Indeed! I'd be making another PR covering these shortly. Thanks.

lpinca commented Mar 18, 2018

Copy link
Copy Markdown
Member

@ryzokuken awesome, thank you.

MylesBorins pushed a commit that referenced this pull request Mar 20, 2018
test: rename regression tests file names
04dc0a9 
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.

- Rename test-regress-GH-io-1068 to test-tty-stdin-end
- Rename test-regress-GH-io-1811 to test-zlib-kmaxlength-rangeerror
- Rename test-regress-GH-node-9326 to test-kill-segfault-freebsd
- Rename test-timers-regress-GH-9765 to test-timers-setimmediate-infinite-loop
- Rename test-tls-pfx-gh-5100-regr to test-tls-pfx-authorizationerror
- Rename test-tls-regr-gh-5108 to test-tls-tlswrap-segfault

PR-URL: #19332
Fixes: #19105
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 20, 2018
test: rename regression tests file names
7df6d9d 
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.

- Rename test-regress-GH-io-1068 to test-tty-stdin-end
- Rename test-regress-GH-io-1811 to test-zlib-kmaxlength-rangeerror
- Rename test-regress-GH-node-9326 to test-kill-segfault-freebsd
- Rename test-timers-regress-GH-9765 to test-timers-setimmediate-infinite-loop
- Rename test-tls-pfx-gh-5100-regr to test-tls-pfx-authorizationerror
- Rename test-tls-regr-gh-5108 to test-tls-tlswrap-segfault

PR-URL: #19332
Fixes: #19105
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
targos mentioned this pull request Mar 20, 2018
Merged
tniessen removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Mar 24, 2018
BethGriggs pushed a commit that referenced this pull request Dec 3, 2018
test: rename regression tests file names
8fa5bd3 
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.

- Rename test-regress-GH-io-1068 to test-tty-stdin-end
- Rename test-regress-GH-io-1811 to test-zlib-kmaxlength-rangeerror
- Rename test-regress-GH-node-9326 to test-kill-segfault-freebsd
- Rename test-timers-regress-GH-9765 to test-timers-setimmediate-infinite-loop
- Rename test-tls-pfx-gh-5100-regr to test-tls-pfx-authorizationerror
- Rename test-tls-regr-gh-5108 to test-tls-tlswrap-segfault

PR-URL: #19332
Fixes: #19105
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
BethGriggs mentioned this pull request Dec 4, 2018
Merged
This was referenced Dec 18, 2018
Closed
Closed
Closed
Closed
Closed
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lpinca lpinca approved these changes
jasnell jasnell approved these changes
richardlau richardlau approved these changes
gireeshpunathil gireeshpunathil approved these changes
+3 more reviewers
Leko Leko approved these changes
hiroppy hiroppy approved these changes
starkwang starkwang approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Back | FazBrowse Home | New Git URL