test: add --repeat option to tools/test.py by mhdawson · Pull Request #6700 · nodejs/node

mhdawson · 2016-05-11T22:01:54Z

Checklist

tests and code linting passes
the commit message follows commit guidelines

Affected core subsystem(s)

test

Description of change

I often want to run a test many times to see if a failure
can be recreated and I believe this is a common
use case. We even have this job in the CI
https://ci.nodejs.org/job/node-stress-single-test/configure
but often you want to run it on a specific machine.

This patch adds the --repeat option so that
you can repeat the selected set of tests a
number of times. Given existing options
in test.py this will allow you to run
one or more tests for the number of
repeats specified. For example:

tools/test.py -j8 --repeat 1000 parallel/test-process-exec-argv

runs the test-process-exec-argv test 1000 times,
running 8 copies in parallel

tools/test.py --repeat 2

would run the entire test suite twice

�

rvagg · 2016-05-12T00:25:30Z

Concept sgtm but I'm not following that Python at all, what's with the new loop that just appends tests? why aren't the arguments to the ListTests() matching up with the arguments in the function definition? Suspecting I'm not at all qualified to sign off on this cause I don't see how it would work.

@jbergstroem? @bnoordhuis? @chrisdickinson?

rvagg · 2016-05-12T00:25:47Z

@thefourtheye you're a Python person too right?

S-YOU · 2016-05-12T01:20:43Z

just fyi, the new loop same as this code

result += tests * repeat

>>> [] * 10
[]
>>> [1] * 10
[1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
>>> [1,2,3] * 10
[1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3]

thefourtheye · 2016-05-12T02:56:04Z

tools/test.py

0 is not needed as it is the default. Moreover xrange is preferred over range in 2.x

mhdawson · 2016-05-12T13:35:08Z

Updated to address all comments now

bnoordhuis · 2016-05-12T15:15:59Z

tools/test.py

Hate to be that guy but... long line. :-)

Perhaps, we can refactor this with dict unpacking.

bnoordhuis · 2016-05-12T15:16:10Z

LGTM with style nit.

thefourtheye · 2016-05-12T15:17:25Z

LGTM

I often want to run a test many times to see if a failure can be recreated and I believe this is a common use case. We even have this job in the CI https://ci.nodejs.org/job/node-stress-single-test/configure but often you want to run it on a specific machine. This patch adds the --repeat option so that you can repeat the selected set of tests a number of times. Given existing options in test.py this will allow you to run one or more tests for the number of repeats specified. For example: tools/test.py -j8 --repeat 1000 parallel/test-process-exec-argv runs the test-process-exec-argv test 1000 times, running 8 copies in parallel tools/test.py --repeat 2 would run the entire test suite twice. �

mhdawson · 2016-05-12T18:07:48Z

Updated for last nit

mhdawson · 2016-05-12T18:09:20Z

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

mhdawson · 2016-05-12T20:09:06Z

CI looks green except for pre-existing issue: #6716 and windows build that seem to be stuck on previous build

mhdawson · 2016-05-12T21:54:34Z

Plan to land tomorrow morning unless there are any objections before then

thefourtheye · 2016-05-13T00:05:09Z

tools/test.py

  result.add_option('--temp-dir',
      help='Optional path to change directory used for tests', default=False)
+  result.add_option('--repeat',
+      help='how many times to repeat each test',


Can this be written as "Number of times to repeat given tests"

jbergstroem · 2016-05-13T01:47:38Z

LGTM with the text update @thefourtheye suggests (or similar).

joaocgreis · 2016-05-13T11:02:17Z

Made sure that --run still works, LGTM.

I often want to run a test many times to see if a failure can be recreated and I believe this is a common use case. We even have this job in the CI https://ci.nodejs.org/job/node-stress-single-test/configure but often you want to run it on a specific machine. This patch adds the --repeat option so that you can repeat the selected set of tests a number of times. Given existing options in test.py this will allow you to run one or more tests for the number of repeats specified. For example: tools/test.py -j8 --repeat 1000 parallel/test-process-exec-argv runs the test-process-exec-argv test 1000 times, running 8 copies in parallel tools/test.py --repeat 2 would run the entire test suite twice. PR-URL: #6700 Reviewed-By: Ben Noorhduis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: thefourtheye - Sakthipriyan Vairamani <[email protected]> Reviewed-By: joaocgreis - João Reis <[email protected]> Reviewed-By: Johan Bergström <[email protected]>

mhdawson · 2016-05-13T13:28:40Z

Ok incorporated updated help message from thefourtheye and landed as ccbb00e

I often want to run a test many times to see if a failure can be recreated and I believe this is a common use case. We even have this job in the CI https://ci.nodejs.org/job/node-stress-single-test/configure but often you want to run it on a specific machine. This patch adds the --repeat option so that you can repeat the selected set of tests a number of times. Given existing options in test.py this will allow you to run one or more tests for the number of repeats specified. For example: tools/test.py -j8 --repeat 1000 parallel/test-process-exec-argv runs the test-process-exec-argv test 1000 times, running 8 copies in parallel tools/test.py --repeat 2 would run the entire test suite twice. PR-URL: #6700 Reviewed-By: Ben Noorhduis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: thefourtheye - Sakthipriyan Vairamani <[email protected]> Reviewed-By: joaocgreis - João Reis <[email protected]> Reviewed-By: Johan Bergström <[email protected]>

MylesBorins · 2016-06-01T22:53:19Z

@mhdawson this will need a manual backport to v4.x

I often want to run a test many times to see if a failure can be recreated and I believe this is a common use case. We even have this job in the CI https://ci.nodejs.org/job/node-stress-single-test/configure but often you want to run it on a specific machine. This patch adds the --repeat option so that you can repeat the selected set of tests a number of times. Given existing options in test.py this will allow you to run one or more tests for the number of repeats specified. For example: tools/test.py -j8 --repeat 1000 parallel/test-process-exec-argv runs the test-process-exec-argv test 1000 times, running 8 copies in parallel tools/test.py --repeat 2 would run the entire test suite twice. PR-URL: #6700 Reviewed-By: Ben Noorhduis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: thefourtheye - Sakthipriyan Vairamani <[email protected]> Reviewed-By: joaocgreis - João Reis <[email protected]> Reviewed-By: Johan Bergström <[email protected]>

MylesBorins · 2016-07-11T20:21:46Z

ping @mhdawson

mhdawson · 2016-07-12T13:57:35Z

Still on my list, will try to do it this week.

Manual backport from master to 4.x stream, original commit message follows. I often want to run a test many times to see if a failure can be recreated and I believe this is a common use case. We even have this job in the CI https://ci.nodejs.org/job/node-stress-single-test/configure but often you want to run it on a specific machine. This patch adds the --repeat option so that you can repeat the selected set of tests a number of times. Given existing options in test.py this will allow you to run one or more tests for the number of repeats specified. For example: tools/test.py -j8 --repeat 1000 parallel/test-process-exec-argv runs the test-process-exec-argv test 1000 times, running 8 copies in parallel tools/test.py --repeat 2 would run the entire test suite twice. PR-URL: nodejs#6700 Reviewed-By: Ben Noorhduis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: thefourtheye - Sakthipriyan Vairamani <[email protected]> Reviewed-By: joaocgreis - João Reis <[email protected]> Reviewed-By: Johan Bergström <[email protected]>

mhdawson · 2016-07-14T23:24:19Z

Ok PR against v4.x-staging to backport #7740

FYI @gibfahn, @thealphanerd

Manual backport from master to 4.x stream, original commit message follows. I often want to run a test many times to see if a failure can be recreated and I believe this is a common use case. We even have this job in the CI https://ci.nodejs.org/job/node-stress-single-test/configure but often you want to run it on a specific machine. This patch adds the --repeat option so that you can repeat the selected set of tests a number of times. Given existing options in test.py this will allow you to run one or more tests for the number of repeats specified. For example: tools/test.py -j8 --repeat 1000 parallel/test-process-exec-argv runs the test-process-exec-argv test 1000 times, running 8 copies in parallel tools/test.py --repeat 2 would run the entire test suite twice. PR-URL: #6700 Reviewed-By: Ben Noorhduis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: thefourtheye - Sakthipriyan Vairamani <[email protected]> Reviewed-By: joaocgreis - João Reis <[email protected]> Reviewed-By: Johan Bergström <[email protected]>

mhdawson added the test Issues and PRs related to the tests. label May 11, 2016

mhdawson self-assigned this May 11, 2016

mhdawson force-pushed the stress branch from 4f3d1a6 to 5343f88 Compare May 11, 2016 22:07

thefourtheye reviewed May 12, 2016
View reviewed changes

tools/test.py Outdated

Copy link

Contributor

thefourtheye May 12, 2016

Choose a reason for hiding this comment

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

0 is not needed as it is the default. Moreover xrange is preferred over range in 2.x

bnoordhuis reviewed May 12, 2016
View reviewed changes

mhdawson added 4 commits May 12, 2016 13:00

address first round of comments

abb3e63

address rest of comments

f36f59d

address another comment

efad8e9

mhdawson force-pushed the stress branch from e31b4c3 to efad8e9 Compare May 12, 2016 18:06

thefourtheye reviewed May 13, 2016
View reviewed changes

mhdawson added land-on-v4.x and removed land-on-v4.x labels May 13, 2016

mhdawson closed this May 13, 2016

mhdawson added the dont-land-on-v6.x label May 13, 2016

mhdawson mentioned this pull request May 17, 2016

2016-05-17, Version 6.2.0 (Stable) #6810

Merged

mhdawson removed the dont-land-on-v6.x label May 17, 2016

mhdawson mentioned this pull request Jul 14, 2016

test: add --repeat option to tools/test.py #7740

Closed

2 tasks

MylesBorins added dont-land-on-v4.x and removed lts-watch-v4.x labels Jul 14, 2016

MylesBorins mentioned this pull request Oct 26, 2016

V4.6.2 proposal #9298

Closed

mhdawson deleted the stress branch March 15, 2017 21:56

Uh oh!

Conversation

mhdawson commented May 11, 2016

Checklist

Affected core subsystem(s)

Description of change

Uh oh!

rvagg commented May 12, 2016

Uh oh!

rvagg commented May 12, 2016

Uh oh!

S-YOU commented May 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thefourtheye May 12, 2016

Choose a reason for hiding this comment

Uh oh!

mhdawson commented May 12, 2016

Uh oh!

bnoordhuis May 12, 2016

Choose a reason for hiding this comment

Uh oh!

thefourtheye May 12, 2016

Choose a reason for hiding this comment

Uh oh!

bnoordhuis commented May 12, 2016

Uh oh!

thefourtheye commented May 12, 2016

Uh oh!

mhdawson commented May 12, 2016

Uh oh!

mhdawson commented May 12, 2016

Uh oh!

mhdawson commented May 12, 2016

Uh oh!

mhdawson commented May 12, 2016

Uh oh!

thefourtheye May 13, 2016

Choose a reason for hiding this comment

Uh oh!

jbergstroem commented May 13, 2016

Uh oh!

joaocgreis commented May 13, 2016

Uh oh!

mhdawson commented May 13, 2016

Uh oh!

MylesBorins commented Jun 1, 2016

Uh oh!

MylesBorins commented Jul 11, 2016

Uh oh!

mhdawson commented Jul 12, 2016

Uh oh!

mhdawson commented Jul 14, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

S-YOU commented May 12, 2016 •

edited

Loading