{"thread":{"id":"50627","subject":"[PATCH 0/2] tests: some touchups related to the --stress feature","startedAt":"2019-03-02T21:19:46Z","lastAt":"2019-03-04T03:55:24Z","messageCount":15,"participants":["Johannes Schindelin via GitGitGadget","Eric Sunshine","Junio C Hamano","SZEDER Gábor","Johannes Schindelin","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"370474","messageId":"pull.155.git.gitgitgadget@gmail.com","threadId":"50627","inReplyTo":null,"subject":"[PATCH 0/2] tests: some touchups related to the --stress feature","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-02T21:19:42Z","receivedAt":"2019-03-02T21:19:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"If my mistake using --stress= instead of --stress-limit= is any indication,\nthen the current options are very confusing.\n\nThis is my attempt at making them less confusing.\n\nJohannes Schindelin (2):\n tests: let --stress-limit= imply --stress\n tests: introduce --stress-jobs=\n\n t/test-lib.sh | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\n\nbase-commit: 7d661e5ed16dca303d7898f5ab0cc2ffc69e0499\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-155%2Fdscho%2Fstress-test-extra-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-155/dscho/stress-test-extra-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/155\n-- \ngitgitgadget\n"},{"id":"370475","messageId":"281d3f1d19d1c93e8d1e66ae16fe3fb286554c0a.1551561582.git.gitgitgadget@gmail.com","threadId":"50627","inReplyTo":"pull.155.git.gitgitgadget@gmail.com","subject":"[PATCH 2/2] tests: introduce --stress-jobs=","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-02T21:19:44Z","receivedAt":"2019-03-02T21:19:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe --stress option currently accepts an argument, but it is confusing\nto at least this user that the argument does not define the maximal\nnumber of stress iterations, but instead the number of jobs to run in\nparallel per stress iteration.\n\nLet's introduce a separate option for that, whose name makes it more\nobvious what it is about, and let --stress= error out with a helpful\nsuggestion about the two options tha could possibly have been meant.\n\nSigned-off-by: Johannes Schindelin \n---\n t/test-lib.sh | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex ab7f27ec6a..6e557982a2 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -142,10 +142,16 @@ do\n \t--stress)\n \t\tstress=t ;;\n \t--stress=*)\n+\t\techo \"error: --stress does not accept an argument: '$opt'\" >&2\n+\t\techo \"did you mean --stress-jobs=${opt#*=} or --stress-limit=${opt#*=}?\" >&2\n+\t\texit 1\n+\t\t;;\n+\t--stress-jobs=*)\n+\t\tstress=t;\n \t\tstress=${opt#--*=}\n \t\tcase \"$stress\" in\n \t\t*[!0-9]*|0*|\"\")\n-\t\t\techo \"error: --stress= requires the number of jobs to run\" >&2\n+\t\t\techo \"error: --stress-jobs= requires the number of jobs to run\" >&2\n \t\t\texit 1\n \t\t\t;;\n \t\t*)\t# Good.\n-- \ngitgitgadget\n"},{"id":"370476","messageId":"16c6508c4b38933945126ffe27445c999aa3e59e.1551561582.git.gitgitgadget@gmail.com","threadId":"50627","inReplyTo":"pull.155.git.gitgitgadget@gmail.com","subject":"[PATCH 1/2] tests: let --stress-limit= imply --stress","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-02T21:19:43Z","receivedAt":"2019-03-02T21:19:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIt does not make much sense that running a test with\n--stress-limit= seemingly ignores that option because it does not\nstress test at all.\n\nSigned-off-by: Johannes Schindelin \n---\n t/test-lib.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 4e7cb52b57..ab7f27ec6a 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -153,6 +153,7 @@ do\n \t\tesac\n \t\t;;\n \t--stress-limit=*)\n+\t\tstress=t;\n \t\tstress_limit=${opt#--*=}\n \t\tcase \"$stress_limit\" in\n \t\t*[!0-9]*|0*|\"\")\n-- \ngitgitgadget\n\n"},{"id":"370499","messageId":"CAPig+cTvj0BL1YjKhpmLVqADxRSb+17aO1+fPmOYPF7Bj_TppQ@mail.gmail.com","threadId":"50627","inReplyTo":"281d3f1d19d1c93e8d1e66ae16fe3fb286554c0a.1551561582.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] tests: introduce --stress-jobs=","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-03-03T02:30:45Z","receivedAt":"2019-03-03T02:30:59Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Mar 2, 2019 at 4:19 PM Johannes Schindelin via GitGitGadget\n wrote:\n> The --stress option currently accepts an argument, but it is confusing\n> to at least this user that the argument does not define the maximal\n> number of stress iterations, but instead the number of jobs to run in\n> parallel per stress iteration.\n>\n> Let's introduce a separate option for that, whose name makes it more\n> obvious what it is about, and let --stress= error out with a helpful\n> suggestion about the two options tha could possibly have been meant.\n\nThis new option probably deserves documentation in t/README alongside\nexisting documentation for --stress and --stress-limit, and the\nexisting --stress= documentation ought be updated.\n\n> Signed-off-by: Johannes Schindelin \n"},{"id":"370502","messageId":"xmqqo96s1dvw.fsf@gitster-ct.c.googlers.com","threadId":"50627","inReplyTo":"16c6508c4b38933945126ffe27445c999aa3e59e.1551561582.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] tests: let --stress-limit= imply --stress","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-03T09:54:59Z","receivedAt":"2019-03-03T09:55:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> It does not make much sense that running a test with\n> --stress-limit= seemingly ignores that option because it does not\n> stress test at all.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> t/test-lib.sh | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 4e7cb52b57..ab7f27ec6a 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -153,6 +153,7 @@ do\n> \t\tesac\n> \t\t;;\n> \t--stress-limit=*)\n> +\t\tstress=t;\n> \t\tstress_limit=${opt#--*=}\n\nGood thinking.\n\n> \t\tcase \"$stress_limit\" in\n> \t\t*[!0-9]*|0*|\"\")\n"},{"id":"370503","messageId":"xmqqh8ck1dvq.fsf@gitster-ct.c.googlers.com","threadId":"50627","inReplyTo":"281d3f1d19d1c93e8d1e66ae16fe3fb286554c0a.1551561582.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] tests: introduce --stress-jobs=","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-03T09:55:05Z","receivedAt":"2019-03-03T09:55:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> The --stress option currently accepts an argument, but it is confusing\n> to at least this user that the argument does not define the maximal\n> number of stress iterations, but instead the number of jobs to run in\n> parallel per stress iteration.\n\nYeah, when there are multiple knobs that can take integral value,\nand especially when the knobs are of equal usefulness, the users\nwould happen pick the right one 50% of the time by accident, which\nis not a happy state.\n\n> Let's introduce a separate option for that, whose name makes it more\n> obvious what it is about, and let --stress= error out with a helpful\n> suggestion about the two options tha could possibly have been meant.\n\nGood. \n\nMaking --stress= error out (instead of deprecating with a\ntransition plan) is probably OK given its audience. It is a good\ntrade-off to save our braincycles by not having to worry about\ntransition and forcing everybody to adjust (I am assuming that\nnobody has a wrapper script to run tXXXX-title.sh scripts that\npasses the --stress= thing).\n\nt/README must be updated as --stress= is documented to specify\nthe degree of parallelism, though.\n\nI'll queue in the meantime, to reduce the risk of forgetting the\ntopic.\n\nThanks.\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> t/test-lib.sh | 8 +++++++-\n> 1 file changed, 7 insertions(+), 1 deletion(-)\n>\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index ab7f27ec6a..6e557982a2 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -142,10 +142,16 @@ do\n> \t--stress)\n> \t\tstress=t ;;\n> \t--stress=*)\n> +\t\techo \"error: --stress does not accept an argument: '$opt'\" >&2\n> +\t\techo \"did you mean --stress-jobs=${opt#*=} or --stress-limit=${opt#*=}?\" >&2\n> +\t\texit 1\n> +\t\t;;\n> +\t--stress-jobs=*)\n> +\t\tstress=t;\n> \t\tstress=${opt#--*=}\n> \t\tcase \"$stress\" in\n> \t\t*[!0-9]*|0*|\"\")\n> -\t\t\techo \"error: --stress= requires the number of jobs to run\" >&2\n> +\t\t\techo \"error: --stress-jobs= requires the number of jobs to run\" >&2\n> \t\t\texit 1\n> \t\t\t;;\n> \t\t*)\t# Good.\n"},{"id":"370522","messageId":"20190303141946.GA28939@szeder.dev","threadId":"50627","inReplyTo":"281d3f1d19d1c93e8d1e66ae16fe3fb286554c0a.1551561582.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] tests: introduce --stress-jobs=","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-03-03T14:19:46Z","receivedAt":"2019-03-03T14:19:57Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sat, Mar 02, 2019 at 01:19:44PM -0800, Johannes Schindelin via GitGitGadget wrote:\n> The --stress option currently accepts an argument, but it is confusing\n> to at least this user that the argument does not define the maximal\n> number of stress iterations, but instead the number of jobs to run in\n> parallel per stress iteration.\n\nWell, these options' description in 't/README' is quite clear on what\nthey do. If the lack of updates to 't/README' in these patches is any\nindication, then you haven't read that :) but doing so might very well\nhave avoided your confusion in the first place.\n\nAccording to my Bash history, I used '--stress='\nabout 20x more often than '--stress-limit='. That's not surprising\nat all, since the main point is to try to trigger rare, hard to\nreproduce failures, no matter how many repetitions it takes. And even\nif there is an upper bound, it is usually not the number of\nrepetitions I know in advance, but rather the time I'm willing to\nsacrifice on it, e.g. how long I'm on lunch break or doing groceries,\nor when I need my CPUs for more important things, or simply when I\ngive up, and hit ctrl-C.\n\nAnd with --stress-jobs= I will have to type more :)\n\n\n> Let's introduce a separate option for that, whose name makes it more\n> obvious what it is about, and let --stress= error out with a helpful\n> suggestion about the two options tha could possibly have been meant.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> t/test-lib.sh | 8 +++++++-\n> 1 file changed, 7 insertions(+), 1 deletion(-)\n> \n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index ab7f27ec6a..6e557982a2 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -142,10 +142,16 @@ do\n> \t--stress)\n> \t\tstress=t ;;\n> \t--stress=*)\n> +\t\techo \"error: --stress does not accept an argument: '$opt'\" >&2\n> +\t\techo \"did you mean --stress-jobs=${opt#*=} or --stress-limit=${opt#*=}?\" >&2\n> +\t\texit 1\n> +\t\t;;\n> +\t--stress-jobs=*)\n> +\t\tstress=t;\n> \t\tstress=${opt#--*=}\n> \t\tcase \"$stress\" in\n> \t\t*[!0-9]*|0*|\"\")\n> -\t\t\techo \"error: --stress= requires the number of jobs to run\" >&2\n> +\t\t\techo \"error: --stress-jobs= requires the number of jobs to run\" >&2\n> \t\t\texit 1\n> \t\t\t;;\n> \t\t*)\t# Good.\n> -- \n> gitgitgadget\n"},{"id":"370523","messageId":"pull.155.v2.git.gitgitgadget@gmail.com","threadId":"50627","inReplyTo":"pull.155.git.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] tests: some touchups related to the --stress feature","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-03T14:44:54Z","receivedAt":"2019-03-03T14:44:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"If my mistake using --stress= instead of --stress-limit= is any indication,\nthen the current options are very confusing.\n\nThis is my attempt at making them less confusing.\n\nChanges since v1:\n\n * Now the patches actually adjust the documentation according to the\n changes ;-)\n\nJohannes Schindelin (2):\n tests: let --stress-limit= imply --stress\n tests: introduce --stress-jobs=\n\n t/README | 8 +++++---\n t/test-lib.sh | 9 ++++++++-\n 2 files changed, 13 insertions(+), 4 deletions(-)\n\n\nbase-commit: 7d661e5ed16dca303d7898f5ab0cc2ffc69e0499\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-155%2Fdscho%2Fstress-test-extra-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-155/dscho/stress-test-extra-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/155\n\nRange-diff vs v1:\n\n 1: 16c6508c4b ! 1: fbe773c22a tests: let --stress-limit= imply --stress\n @@ -8,6 +8,19 @@\n \n Signed-off-by: Johannes Schindelin \n \n + diff --git a/t/README b/t/README\n + --- a/t/README\n + +++ b/t/README\n +@@\n + --stress-limit=::\n + \tWhen combined with --stress run the test script repeatedly\n + \tthis many times in each of the parallel jobs or until one of\n +-\tthem fails, whichever comes first.\n ++\tthem fails, whichever comes first. Implies `--stress`.\n + \n + You can also set the GIT_TEST_INSTALLED environment variable to\n + the bindir of an existing git installation to test that installation.\n +\n diff --git a/t/test-lib.sh b/t/test-lib.sh\n --- a/t/test-lib.sh\n +++ b/t/test-lib.sh\n 2: 281d3f1d19 ! 2: 074628c22b tests: introduce --stress-jobs=\n @@ -13,6 +13,33 @@\n \n Signed-off-by: Johannes Schindelin \n \n + diff --git a/t/README b/t/README\n + --- a/t/README\n + +++ b/t/README\n +@@\n + \tvariable to \"1\" or \"0\", respectively.\n + \n + --stress::\n +---stress=::\n + \tRun the test script repeatedly in multiple parallel jobs until\n + \tone of them fails. Useful for reproducing rare failures in\n + \tflaky tests. The number of parallel jobs is, in order of\n +-\tprecedence: , or the value of the GIT_TEST_STRESS_LOAD\n ++\tprecedence: the value of the GIT_TEST_STRESS_LOAD\n + \tenvironment variable, or twice the number of available\n + \tprocessors (as shown by the 'getconf' utility),\tor 8.\n + \tImplies `--verbose -x --immediate` to get the most information\n +@@\n + \t'.stress-' suffix, and the trash directory of the failed\n + \ttest job is renamed to end with a '.stress-failed' suffix.\n + \n ++--stress-jobs=::\n ++\tOverride the number of parallel jobs. Implies `--stress`.\n ++\n + --stress-limit=::\n + \tWhen combined with --stress run the test script repeatedly\n + \tthis many times in each of the parallel jobs or until one of\n +\n diff --git a/t/test-lib.sh b/t/test-lib.sh\n --- a/t/test-lib.sh\n +++ b/t/test-lib.sh\n\n-- \ngitgitgadget\n"},{"id":"370524","messageId":"fbe773c22a4ccea3e4eca8c1bd0206941d23f590.1551624293.git.gitgitgadget@gmail.com","threadId":"50627","inReplyTo":"pull.155.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] tests: let --stress-limit= imply --stress","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-03T14:44:54Z","receivedAt":"2019-03-03T14:44:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIt does not make much sense that running a test with\n--stress-limit= seemingly ignores that option because it does not\nstress test at all.\n\nSigned-off-by: Johannes Schindelin \n---\n t/README | 2 +-\n t/test-lib.sh | 1 +\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/README b/t/README\nindex 3aed321248..b61bc930c4 100644\n--- a/t/README\n+++ b/t/README\n@@ -205,7 +205,7 @@ appropriately before running \"make\".\n --stress-limit=::\n \tWhen combined with --stress run the test script repeatedly\n \tthis many times in each of the parallel jobs or until one of\n-\tthem fails, whichever comes first.\n+\tthem fails, whichever comes first. Implies `--stress`.\n \n You can also set the GIT_TEST_INSTALLED environment variable to\n the bindir of an existing git installation to test that installation.\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 4e7cb52b57..ab7f27ec6a 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -153,6 +153,7 @@ do\n \t\tesac\n \t\t;;\n \t--stress-limit=*)\n+\t\tstress=t;\n \t\tstress_limit=${opt#--*=}\n \t\tcase \"$stress_limit\" in\n \t\t*[!0-9]*|0*|\"\")\n-- \ngitgitgadget\n\n"},{"id":"370525","messageId":"074628c22b2df82280b06db604196f25300e8f87.1551624293.git.gitgitgadget@gmail.com","threadId":"50627","inReplyTo":"pull.155.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] tests: introduce --stress-jobs=","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-03T14:44:55Z","receivedAt":"2019-03-03T14:45:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe --stress option currently accepts an argument, but it is confusing\nto at least this user that the argument does not define the maximal\nnumber of stress iterations, but instead the number of jobs to run in\nparallel per stress iteration.\n\nLet's introduce a separate option for that, whose name makes it more\nobvious what it is about, and let --stress= error out with a helpful\nsuggestion about the two options tha could possibly have been meant.\n\nSigned-off-by: Johannes Schindelin \n---\n t/README | 6 ++++--\n t/test-lib.sh | 8 +++++++-\n 2 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex b61bc930c4..a496be56ef 100644\n--- a/t/README\n+++ b/t/README\n@@ -187,11 +187,10 @@ appropriately before running \"make\".\n \tvariable to \"1\" or \"0\", respectively.\n \n --stress::\n---stress=::\n \tRun the test script repeatedly in multiple parallel jobs until\n \tone of them fails. Useful for reproducing rare failures in\n \tflaky tests. The number of parallel jobs is, in order of\n-\tprecedence: , or the value of the GIT_TEST_STRESS_LOAD\n+\tprecedence: the value of the GIT_TEST_STRESS_LOAD\n \tenvironment variable, or twice the number of available\n \tprocessors (as shown by the 'getconf' utility),\tor 8.\n \tImplies `--verbose -x --immediate` to get the most information\n@@ -202,6 +201,9 @@ appropriately before running \"make\".\n \t'.stress-' suffix, and the trash directory of the failed\n \ttest job is renamed to end with a '.stress-failed' suffix.\n \n+--stress-jobs=::\n+\tOverride the number of parallel jobs. Implies `--stress`.\n+\n --stress-limit=::\n \tWhen combined with --stress run the test script repeatedly\n \tthis many times in each of the parallel jobs or until one of\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex ab7f27ec6a..6e557982a2 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -142,10 +142,16 @@ do\n \t--stress)\n \t\tstress=t ;;\n \t--stress=*)\n+\t\techo \"error: --stress does not accept an argument: '$opt'\" >&2\n+\t\techo \"did you mean --stress-jobs=${opt#*=} or --stress-limit=${opt#*=}?\" >&2\n+\t\texit 1\n+\t\t;;\n+\t--stress-jobs=*)\n+\t\tstress=t;\n \t\tstress=${opt#--*=}\n \t\tcase \"$stress\" in\n \t\t*[!0-9]*|0*|\"\")\n-\t\t\techo \"error: --stress= requires the number of jobs to run\" >&2\n+\t\t\techo \"error: --stress-jobs= requires the number of jobs to run\" >&2\n \t\t\texit 1\n \t\t\t;;\n \t\t*)\t# Good.\n-- \ngitgitgadget\n"},{"id":"370526","messageId":"nycvar.QRO.7.76.6.1903031544310.45@tvgsbejvaqbjf.bet","threadId":"50627","inReplyTo":"20190303141946.GA28939@szeder.dev","subject":"Re: [PATCH 2/2] tests: introduce --stress-jobs=","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-03-03T14:47:01Z","receivedAt":"2019-03-03T14:47:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 3 Mar 2019, SZEDER Gábor wrote:\n\n> On Sat, Mar 02, 2019 at 01:19:44PM -0800, Johannes Schindelin via GitGitGadget wrote:\n> > The --stress option currently accepts an argument, but it is confusing\n> > to at least this user that the argument does not define the maximal\n> > number of stress iterations, but instead the number of jobs to run in\n> > parallel per stress iteration.\n> \n> Well, these options' description in 't/README' is quite clear on what\n> they do. If the lack of updates to 't/README' in these patches is any\n> indication, then you haven't read that :) but doing so might very well\n> have avoided your confusion in the first place.\n\nYep, hadn't read it, assumed that it was obvious that `--stress=`\nmeant: try for at most times to replicate the issue.\n\nWhich to me is a good indicator that the UI could be improved...\n\n> According to my Bash history, I used '--stress='\n> about 20x more often than '--stress-limit='. That's not surprising\n> at all, since the main point is to try to trigger rare, hard to\n> reproduce failures, no matter how many repetitions it takes. And even\n> if there is an upper bound, it is usually not the number of repetitions\n> I know in advance, but rather the time I'm willing to sacrifice on it,\n> e.g. how long I'm on lunch break or doing groceries, or when I need my\n> CPUs for more important things, or simply when I give up, and hit\n> ctrl-C.\n\nI don't doubt that *you* need `--stress-jobs=` more often than\n`--stress-limit=`, but I really think that common users will need the\nlatter a lot more often (if they need the former at all).\n\n> And with --stress-jobs= I will have to type more :)\n\nSorry ;-)\n\nCiao,\nDscho"},{"id":"370528","messageId":"CAPig+cS1URdnRb0RM0HYQhtmXWn5knx6Ee1Y96Gc9dt_9LmRKA@mail.gmail.com","threadId":"50627","inReplyTo":"074628c22b2df82280b06db604196f25300e8f87.1551624293.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/2] tests: introduce --stress-jobs=","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-03-03T15:00:32Z","receivedAt":"2019-03-03T15:00:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Mar 3, 2019 at 9:45 AM Johannes Schindelin via GitGitGadget\n wrote:\n> [...]\n> Let's introduce a separate option for that, whose name makes it more\n> obvious what it is about, and let --stress= error out with a helpful\n> suggestion about the two options tha could possibly have been meant.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> diff --git a/t/README b/t/README\n> @@ -187,11 +187,10 @@ appropriately before running \"make\".\n> variable to \"1\" or \"0\", respectively.\n>\n> --stress::\n> ---stress=::\n\nShouldn't the \"--stress=\" line be removed?\n\n> Run the test script repeatedly in multiple parallel jobs until\n> one of them fails. Useful for reproducing rare failures in\n> flaky tests. The number of parallel jobs is, in order of\n> - precedence: , or the value of the GIT_TEST_STRESS_LOAD\n> + precedence: the value of the GIT_TEST_STRESS_LOAD\n> environment variable, or twice the number of available\n> processors (as shown by the 'getconf' utility), or 8.\n"},{"id":"370550","messageId":"20190303174501.GG23811@sigill.intra.peff.net","threadId":"50627","inReplyTo":"074628c22b2df82280b06db604196f25300e8f87.1551624293.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/2] tests: introduce --stress-jobs=","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-03T17:45:01Z","receivedAt":"2019-03-03T17:45:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 03, 2019 at 06:44:55AM -0800, Johannes Schindelin via GitGitGadget wrote:\n\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index ab7f27ec6a..6e557982a2 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -142,10 +142,16 @@ do\n> \t--stress)\n> \t\tstress=t ;;\n> \t--stress=*)\n> +\t\techo \"error: --stress does not accept an argument: '$opt'\" >&2\n> +\t\techo \"did you mean --stress-jobs=${opt#*=} or --stress-limit=${opt#*=}?\" >&2\n> +\t\texit 1\n\nThis seems reasonable. I was all set to argue that \"--stress-limit\" is\nmuch more common than \"--stress-jobs\", but I see Gábor just made the\nopposite argument. So maybe just informing the user is the right thing. :)\n\n-Peff\n"},{"id":"370567","messageId":"xmqqfts3z5kw.fsf@gitster-ct.c.googlers.com","threadId":"50627","inReplyTo":"CAPig+cS1URdnRb0RM0HYQhtmXWn5knx6Ee1Y96Gc9dt_9LmRKA@mail.gmail.com","subject":"Re: [PATCH v2 2/2] tests: introduce --stress-jobs=","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-04T03:22:39Z","receivedAt":"2019-03-04T03:22:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Sun, Mar 3, 2019 at 9:45 AM Johannes Schindelin via GitGitGadget\n> wrote:\n>> [...]\n>> Let's introduce a separate option for that, whose name makes it more\n>> obvious what it is about, and let --stress= error out with a helpful\n>> suggestion about the two options tha could possibly have been meant.\n>>\n>> Signed-off-by: Johannes Schindelin \n>> ---\n>> diff --git a/t/README b/t/README\n>> @@ -187,11 +187,10 @@ appropriately before running \"make\".\n>> variable to \"1\" or \"0\", respectively.\n>>\n>> --stress::\n>> ---stress=::\n>\n> Shouldn't the \"--stress=\" line be removed?\n\nEyes can easily be tricked by the patch format, but the above hunk\ndoes remove that line ;-) I had the same reaction when I saw your\nmessage for the first time (before seeing the patch itself).\n\n>\n>> Run the test script repeatedly in multiple parallel jobs until\n>> one of them fails. Useful for reproducing rare failures in\n>> flaky tests. The number of parallel jobs is, in order of\n>> - precedence: , or the value of the GIT_TEST_STRESS_LOAD\n>> + precedence: the value of the GIT_TEST_STRESS_LOAD\n>> environment variable, or twice the number of available\n>> processors (as shown by the 'getconf' utility), or 8.\n"},{"id":"370570","messageId":"CAPig+cR8ukjA+mYR2zJBt-Q23SS2HJeBF0v_iEYY8-7JGRRh=Q@mail.gmail.com","threadId":"50627","inReplyTo":"xmqqfts3z5kw.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 2/2] tests: introduce --stress-jobs=","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-03-04T03:55:09Z","receivedAt":"2019-03-04T03:55:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Mar 3, 2019 at 10:22 PM Junio C Hamano wrote:\n> Eric Sunshine writes:\n> > On Sun, Mar 3, 2019 at 9:45 AM Johannes Schindelin via GitGitGadget\n> > wrote:\n> >> ---stress=::\n> >\n> > Shouldn't the \"--stress=\" line be removed?\n>\n> Eyes can easily be tricked by the patch format, but the above hunk\n> does remove that line ;-) I had the same reaction when I saw your\n> message for the first time (before seeing the patch itself).\n\nYep, my eyes were tricked. Thanks for pointing it out, and sorry for the noise.\n"}]}