| Message ID | 074628c22b2df82280b06db604196f25300e8f87.1551624293.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests: some touchups related to the --stress feature | expand |
On Sun, Mar 3, 2019 at 9:45 AM Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com> wrote: > [...] > Let's introduce a separate option for that, whose name makes it more > obvious what it is about, and let --stress=<N> error out with a helpful > suggestion about the two options tha could possibly have been meant. > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > --- > diff --git a/t/README b/t/README > @@ -187,11 +187,10 @@ appropriately before running "make". > variable to "1" or "0", respectively. > > --stress:: > ---stress=<N>:: Shouldn't the "--stress=<N>" line be removed? > Run the test script repeatedly in multiple parallel jobs until > one of them fails. Useful for reproducing rare failures in > flaky tests. The number of parallel jobs is, in order of > - precedence: <N>, or the value of the GIT_TEST_STRESS_LOAD > + precedence: the value of the GIT_TEST_STRESS_LOAD > environment variable, or twice the number of available > processors (as shown by the 'getconf' utility), or 8.
On Sun, Mar 03, 2019 at 06:44:55AM -0800, Johannes Schindelin via GitGitGadget wrote: > diff --git a/t/test-lib.sh b/t/test-lib.sh > index ab7f27ec6a..6e557982a2 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -142,10 +142,16 @@ do > --stress) > stress=t ;; > --stress=*) > + echo "error: --stress does not accept an argument: '$opt'" >&2 > + echo "did you mean --stress-jobs=${opt#*=} or --stress-limit=${opt#*=}?" >&2 > + exit 1 This seems reasonable. I was all set to argue that "--stress-limit" is much more common than "--stress-jobs", but I see Gábor just made the opposite argument. So maybe just informing the user is the right thing. :) -Peff
Eric Sunshine <sunshine@sunshineco.com> writes: > On Sun, Mar 3, 2019 at 9:45 AM Johannes Schindelin via GitGitGadget > <gitgitgadget@gmail.com> wrote: >> [...] >> Let's introduce a separate option for that, whose name makes it more >> obvious what it is about, and let --stress=<N> error out with a helpful >> suggestion about the two options tha could possibly have been meant. >> >> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> >> --- >> diff --git a/t/README b/t/README >> @@ -187,11 +187,10 @@ appropriately before running "make". >> variable to "1" or "0", respectively. >> >> --stress:: >> ---stress=<N>:: > > Shouldn't the "--stress=<N>" line be removed? Eyes can easily be tricked by the patch format, but the above hunk does remove that line ;-) I had the same reaction when I saw your message for the first time (before seeing the patch itself). > >> Run the test script repeatedly in multiple parallel jobs until >> one of them fails. Useful for reproducing rare failures in >> flaky tests. The number of parallel jobs is, in order of >> - precedence: <N>, or the value of the GIT_TEST_STRESS_LOAD >> + precedence: the value of the GIT_TEST_STRESS_LOAD >> environment variable, or twice the number of available >> processors (as shown by the 'getconf' utility), or 8.
On Sun, Mar 3, 2019 at 10:22 PM Junio C Hamano <gitster@pobox.com> wrote: > Eric Sunshine <sunshine@sunshineco.com> writes: > > On Sun, Mar 3, 2019 at 9:45 AM Johannes Schindelin via GitGitGadget > > <gitgitgadget@gmail.com> wrote: > >> ---stress=<N>:: > > > > Shouldn't the "--stress=<N>" line be removed? > > Eyes can easily be tricked by the patch format, but the above hunk > does remove that line ;-) I had the same reaction when I saw your > message for the first time (before seeing the patch itself). Yep, my eyes were tricked. Thanks for pointing it out, and sorry for the noise.
diff --git a/t/README b/t/README index b61bc930c4..a496be56ef 100644 --- a/t/README +++ b/t/README @@ -187,11 +187,10 @@ appropriately before running "make". variable to "1" or "0", respectively. --stress:: ---stress=<N>:: Run the test script repeatedly in multiple parallel jobs until one of them fails. Useful for reproducing rare failures in flaky tests. The number of parallel jobs is, in order of - precedence: <N>, or the value of the GIT_TEST_STRESS_LOAD + precedence: the value of the GIT_TEST_STRESS_LOAD environment variable, or twice the number of available processors (as shown by the 'getconf' utility), or 8. Implies `--verbose -x --immediate` to get the most information @@ -202,6 +201,9 @@ appropriately before running "make". '.stress-<nr>' suffix, and the trash directory of the failed test job is renamed to end with a '.stress-failed' suffix. +--stress-jobs=<N>:: + Override the number of parallel jobs. Implies `--stress`. + --stress-limit=<N>:: When combined with --stress run the test script repeatedly this many times in each of the parallel jobs or until one of diff --git a/t/test-lib.sh b/t/test-lib.sh index ab7f27ec6a..6e557982a2 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -142,10 +142,16 @@ do --stress) stress=t ;; --stress=*) + echo "error: --stress does not accept an argument: '$opt'" >&2 + echo "did you mean --stress-jobs=${opt#*=} or --stress-limit=${opt#*=}?" >&2 + exit 1 + ;; + --stress-jobs=*) + stress=t; stress=${opt#--*=} case "$stress" in *[!0-9]*|0*|"") - echo "error: --stress=<N> requires the number of jobs to run" >&2 + echo "error: --stress-jobs=<N> requires the number of jobs to run" >&2 exit 1 ;; *) # Good.