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

ryzokuken · 2018-03-13T21:48:50Z

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

ryzokuken · 2018-03-13T21:49:42Z

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

joyeecheung · 2018-03-14T13:21:27Z

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

lpinca · 2018-03-14T16:01:32Z

@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 · 2018-03-15T21:18:49Z

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

ryzokuken · 2018-03-15T21:20:34Z

@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 · 2018-03-16T11:49:49Z

test/parallel/test-kill-segfault-freebsd.js

@@ -1,5 +1,10 @@
 'use strict';
 require('../common');
+
+// This test ensures Node doesn't crash on hitting Ctrl+C in order to terminate


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

lpinca · 2018-03-16T11:51:10Z

test/parallel/test-tls-tlswrap-segfault.js

 if (!common.hasCrypto)
  common.skip('missing crypto');
+const fixtures = require('../common/fixtures');
+
+// This test ensures that Node doesn't incur a segfault while accessing TLSWrap


lpinca · 2018-03-16T11:51:38Z

test/parallel/test-tty-stdin-end.js

+'use strict';
+require('../common');
+
+// This test ensures that node doesn't crash on `process.stdin.emit("end")`.


Same thing.

ryzokuken · 2018-03-16T12:16:31Z

@lpinca done.

lpinca · 2018-03-16T12:23:42Z

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

lpinca · 2018-03-16T13:57:38Z

test/parallel/test-zlib-kmaxlength-rangeerror.js


 // Change kMaxLength for zlib to trigger the error without having to allocate
 // large Buffers.
-const buffer = require('buffer');


@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.

My bad. Fixing this in a sec.

ryzokuken · 2018-03-17T12:09:30Z

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

lpinca · 2018-03-17T12:44:28Z

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

ryzokuken · 2018-03-17T13:45:14Z

@lpinca they passed! 🎉

lpinca · 2018-03-18T13:38:15Z

@ryzokuken would you mind squashing commits? Thank you.

ryzokuken · 2018-03-18T14:11:24Z

@lpinca sure! Just give me a second.

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 · 2018-03-18T14:18:02Z

@lpinca looks good enough?

lpinca · 2018-03-18T14:24:40Z

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

ryzokuken · 2018-03-18T16:18:47Z

@lpinca any idea what could've gone wrong?

lpinca · 2018-03-18T16:48:23Z

AIX failure is not related, will land this later.

lpinca · 2018-03-18T18:21:23Z

Landed in 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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Weijia Wang <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Shingo Inoue <[email protected]>

lpinca · 2018-03-18T18:32:19Z

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 · 2018-03-18T18:33:20Z

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

lpinca · 2018-03-18T18:34:37Z

@ryzokuken awesome, thank you.

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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Weijia Wang <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Shingo Inoue <[email protected]>

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

gireeshpunathil approved these changes Mar 14, 2018

View reviewed changes

jasnell approved these changes Mar 14, 2018

View reviewed changes

starkwang approved these changes Mar 14, 2018

View reviewed changes

hiroppy approved these changes Mar 14, 2018

View reviewed changes

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

richardlau approved these changes Mar 16, 2018

View reviewed changes

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

lpinca added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 16, 2018

lpinca reviewed Mar 16, 2018

View reviewed changes

lpinca removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 16, 2018

Leko approved these changes Mar 17, 2018

View reviewed changes

lpinca added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 18, 2018

ryzokuken force-pushed the test/regress branch from b1379a9 to 1db4202 Compare March 18, 2018 14:16

lpinca closed this Mar 18, 2018

targos mentioned this pull request Mar 20, 2018

v9.9.0 proposal #19428

Merged

tniessen removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 24, 2018

BethGriggs added the land-on-v8.x label Dec 3, 2018

BethGriggs mentioned this pull request Dec 4, 2018

v8.14.1 proposal #24832

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

ryzokuken commented Mar 13, 2018 •

edited

Loading

ryzokuken commented Mar 13, 2018

joyeecheung commented Mar 14, 2018

lpinca commented Mar 14, 2018

ryzokuken commented Mar 15, 2018 •

edited

Loading

ryzokuken commented Mar 15, 2018

lpinca Mar 16, 2018

lpinca Mar 16, 2018

lpinca Mar 16, 2018

ryzokuken commented Mar 16, 2018

lpinca commented Mar 16, 2018

lpinca Mar 16, 2018 •

edited

Loading

ryzokuken Mar 17, 2018

ryzokuken commented Mar 17, 2018

lpinca commented Mar 17, 2018

ryzokuken commented Mar 17, 2018

lpinca commented Mar 18, 2018

ryzokuken commented Mar 18, 2018

ryzokuken commented Mar 18, 2018

lpinca commented Mar 18, 2018

ryzokuken commented Mar 18, 2018

lpinca commented Mar 18, 2018

lpinca commented Mar 18, 2018

lpinca commented Mar 18, 2018

ryzokuken commented Mar 18, 2018

lpinca commented Mar 18, 2018

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

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

Conversation

ryzokuken commented Mar 13, 2018 • edited Loading

Checklist

Additional files renamed

ryzokuken commented Mar 13, 2018

joyeecheung commented Mar 14, 2018

lpinca commented Mar 14, 2018

ryzokuken commented Mar 15, 2018 • edited Loading

ryzokuken commented Mar 15, 2018

lpinca Mar 16, 2018

Choose a reason for hiding this comment

lpinca Mar 16, 2018

Choose a reason for hiding this comment

lpinca Mar 16, 2018

Choose a reason for hiding this comment

ryzokuken commented Mar 16, 2018

lpinca commented Mar 16, 2018

lpinca Mar 16, 2018 • edited Loading

Choose a reason for hiding this comment

ryzokuken Mar 17, 2018

Choose a reason for hiding this comment

ryzokuken commented Mar 17, 2018

lpinca commented Mar 17, 2018

ryzokuken commented Mar 17, 2018

lpinca commented Mar 18, 2018

ryzokuken commented Mar 18, 2018

ryzokuken commented Mar 18, 2018

lpinca commented Mar 18, 2018

ryzokuken commented Mar 18, 2018

lpinca commented Mar 18, 2018

lpinca commented Mar 18, 2018

lpinca commented Mar 18, 2018

ryzokuken commented Mar 18, 2018

lpinca commented Mar 18, 2018

ryzokuken commented Mar 13, 2018 •

edited

Loading

ryzokuken commented Mar 15, 2018 •

edited

Loading

lpinca Mar 16, 2018 •

edited

Loading