test: rename regression tests with descriptive file names by ryzokuken · Pull Request #19161 · nodejs/node

ryzokuken · 2018-03-06T05:35:53Z

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

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

Affected core subsystem(s)

test

Checklist

test/sequential/test-regress-GH-1697.js
test/sequential/test-regress-GH-1726.js
test/sequential/test-regress-GH-4015.js
test/sequential/test-regress-GH-4027.js
test/sequential/test-regress-GH-784.js
test/sequential/test-regress-GH-877.js

ryzokuken · 2018-03-06T06:15:03Z

@joyeecheung @BridgeAR please take a look

richardlau

Good start. Big 👍 for also updating to conform to the standard test structure.

richardlau · 2018-03-06T10:08:24Z

test/sequential/test-regress-fs-stat_sync.js

Maybe rename the fixture used a few lines down for consistency?

richardlau · 2018-03-06T10:11:06Z

test/sequential/test-regress-net-response_size.js

Small nit: Windows

joyeecheung · 2018-03-06T11:52:00Z

Thanks for doing this! At this point I think we can drop the regress in the file names, names like just test-net-response-size.js should be fine.

EDIT: oops forgot to change the file name

joyeecheung · 2018-03-06T11:55:13Z

test/sequential/test-regress-fs-stat_sync.js

This file can be named test-fs-statSync-overflow.js since it is testing that specific behavior of fs.statSync.

Also, can you rename the fixtures used below as well? Thanks

joyeecheung · 2018-03-06T11:56:59Z

test/sequential/test-regress-fs-watch_file.js

test-regress-fs-watch_file.js seems a bit general. Maybe name this test-fs-watchFile-enoent-after-deletion?

joyeecheung · 2018-03-06T11:59:01Z

test/sequential/test-regress-net-response_size.js

Can you use hyphens instead of underscores in the file name for consistency? Also for camel cases I think they can either stay in the name as camelCase or be made snake cases like camel-case

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

Rename affected tests to remove the "regress" keyword as well as improve naming scheme. Refs: nodejs#19105

Refs: nodejs#19105

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

BridgeAR · 2018-03-06T17:49:33Z

I removed the "fixes" part in the description. This does fix all sequential entries but not the parallel ones.

BridgeAR · 2018-03-06T17:50:51Z

Lite-CI https://ci.nodejs.org/job/node-test-pull-request-lite/227/

ryzokuken · 2018-03-06T17:56:51Z

Thanks, @BridgeAR! You're absolutely right. I should've paid more attention and done it myself, sorry.

I talked to @joyeecheung and we decided to break the original issue (#19105) plus a few other tests that were differently named (but somewhat on the same lines) and were missed into multiple PRs, so as to make sure that the PRs weren't too large.

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>

Refs: #19105 PR-URL: #19161 Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure 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>

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>

Rename affected tests to remove the "regress" keyword as well as improve naming scheme. Refs: #19105 PR-URL: #19161 Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure 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>

Refs: #19105 PR-URL: #19161 Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure 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>

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>

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>

Rename affected tests to remove the "regress" keyword as well as improve naming scheme. Refs: nodejs#19105 PR-URL: nodejs#19161 Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure 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>

Refs: nodejs#19105 PR-URL: nodejs#19161 Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure 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>

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>

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>

Rename affected tests to remove the "regress" keyword as well as improve naming scheme. Refs: #19105 PR-URL: #19161 Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure 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>

Refs: #19105 PR-URL: #19161 Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure 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>

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>

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

richardlau reviewed Mar 6, 2018

View reviewed changes

joyeecheung reviewed Mar 6, 2018

View reviewed changes

ryzokuken added 5 commits March 6, 2018 18:21

test: rename tests to remove "regress" keyword

24016c4

Rename affected tests to remove the "regress" keyword as well as improve naming scheme. Refs: nodejs#19105

ryzokuken force-pushed the test/rename-regression branch from 6b700b1 to 24016c4 Compare March 6, 2018 13:41

ryzokuken added 3 commits March 6, 2018 19:29

test: address nits and rename the corresponding fixture

5b96920

Refs: nodejs#19105

ryzokuken changed the title ~~[WIP] test: rename regression tests with descriptive file names~~ test: rename regression tests with descriptive file names Mar 6, 2018

jasnell approved these changes Mar 6, 2018

View reviewed changes

BridgeAR approved these changes Mar 6, 2018

View reviewed changes

BridgeAR added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. labels Mar 6, 2018

joyeecheung approved these changes Mar 6, 2018

View reviewed changes

richardlau approved these changes Mar 7, 2018

View reviewed changes

gireeshpunathil approved these changes Mar 7, 2018

View reviewed changes

targos mentioned this pull request Mar 18, 2018

v9.9.0 proposal #19428

Merged

MylesBorins added the land-on-v8.x label Aug 17, 2018

MylesBorins mentioned this pull request Aug 17, 2018

v8.12.0 proposal #21593

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test: rename regression tests with descriptive file names#19161

test: rename regression tests with descriptive file names#19161
ryzokuken wants to merge 8 commits intonodejs:masterfrom
ryzokuken:test/rename-regression

ryzokuken commented Mar 6, 2018 •

edited by BridgeAR

Loading

Uh oh!

ryzokuken commented Mar 6, 2018

Uh oh!

richardlau left a comment

Uh oh!

richardlau Mar 6, 2018

Uh oh!

richardlau Mar 6, 2018

Uh oh!

joyeecheung commented Mar 6, 2018 •

edited

Loading

Uh oh!

joyeecheung Mar 6, 2018

Uh oh!

joyeecheung Mar 6, 2018

Uh oh!

joyeecheung Mar 6, 2018 •

edited

Loading

Uh oh!

BridgeAR commented Mar 6, 2018

Uh oh!

BridgeAR commented Mar 6, 2018

Uh oh!

ryzokuken commented Mar 6, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Uh oh!

Conversation

ryzokuken commented Mar 6, 2018 • edited by BridgeAR Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Affected core subsystem(s)

Checklist

Uh oh!

ryzokuken commented Mar 6, 2018

Uh oh!

richardlau left a comment

Choose a reason for hiding this comment

Uh oh!

richardlau Mar 6, 2018

Choose a reason for hiding this comment

Uh oh!

richardlau Mar 6, 2018

Choose a reason for hiding this comment

Uh oh!

joyeecheung commented Mar 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joyeecheung Mar 6, 2018

Choose a reason for hiding this comment

Uh oh!

joyeecheung Mar 6, 2018

Choose a reason for hiding this comment

Uh oh!

joyeecheung Mar 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BridgeAR commented Mar 6, 2018

Uh oh!

BridgeAR commented Mar 6, 2018

Uh oh!

ryzokuken commented Mar 6, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

ryzokuken commented Mar 6, 2018 •

edited by BridgeAR

Loading

joyeecheung commented Mar 6, 2018 •

edited

Loading

joyeecheung Mar 6, 2018 •

edited

Loading