| Message ID | 20181209225628.22216-1-szeder.dev@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | test-lib: add the '--stress' option to help reproduce occasional failures in flaky tests | expand |
On Sun, Dec 09, 2018 at 11:56:21PM +0100, SZEDER Gábor wrote: > This patch series tries to make reproducing rare failures in flaky > tests easier: it adds the '--stress' option to our test library to run > the test script repeatedly in multiple parallel jobs, in the hope that > the increased load creates enough variance in the timing of the test's > commands that such a failure is eventually triggered. > > Notable changes since v1: > > - Made it more Peff-friendly :), namely it will now show the log of > the failed test and will rename its trash directory. > > Furthermore, '--stress' will now imply '--verbose -x --immediate'. :) Thanks. I do sympathize with the notion of keeping orthogonal things orthogonal, but I hope that this will make the tool a little more pleasant to use out of the box. We can always tweak the behavior later, too. It's not like this is something user-facing that we've promised as a scripting interface. > - Improved abort handling based on the discussion of the previous > version. (As a result, the patch is so heavily modified, that > 'range-diff' with default parameters consideres it a different > patch; increasing the creation factor then results in one big ugly > diff of a diff, so I left it as-is.) Yeah, this all looked good to me. > - Added a few new patches, mostly preparatory refactorings, though > the first one might be considered a bugfix. I left a few minor comments, but these all looked good to me. -Peff
This patch series tries to make reproducing rare failures in flaky tests easier: it adds the '--stress' option to our test library to run the test script repeatedly in multiple parallel jobs, in the hope that the increased load creates enough variance in the timing of the test's commands that such a failure is eventually triggered. Notable changes since v1: - Made it more Peff-friendly :), namely it will now show the log of the failed test and will rename its trash directory. Furthermore, '--stress' will now imply '--verbose -x --immediate'. - Improved abort handling based on the discussion of the previous version. (As a result, the patch is so heavily modified, that 'range-diff' with default parameters consideres it a different patch; increasing the creation factor then results in one big ugly diff of a diff, so I left it as-is.) - Added a few new patches, mostly preparatory refactorings, though the first one might be considered a bugfix. - Other minor cleanups suggested in the previous discussion. v1: https://public-inbox.org/git/20181204163457.15717-1-szeder.dev@gmail.com/T/ SZEDER Gábor (7): test-lib: translate SIGTERM and SIGHUP to an exit test-lib: parse some --options earlier test-lib: consolidate naming of test-results paths test-lib: set $TRASH_DIRECTORY earlier test-lib: extract Bash version check for '-x' tracing test-lib-functions: introduce the 'test_set_port' helper function test-lib: add the '--stress' option to run a test repeatedly under load t/README | 19 +++- t/lib-git-daemon.sh | 2 +- t/lib-git-p4.sh | 9 +- t/lib-git-svn.sh | 2 +- t/lib-httpd.sh | 2 +- t/t0410-partial-clone.sh | 1 - t/t5512-ls-remote.sh | 2 +- t/test-lib-functions.sh | 39 +++++++ t/test-lib.sh | 227 +++++++++++++++++++++++++++++---------- 9 files changed, 230 insertions(+), 73 deletions(-) Range-diff: -: ---------- > 1: 3a5c926167 test-lib: translate SIGTERM and SIGHUP to an exit -: ---------- > 2: 8eee8d7fba test-lib: parse some --options earlier 1: f4bb53e676 ! 3: dd20ae5e9a test-lib: consolidate naming of test-results paths @@ -4,8 +4,9 @@ There are two places where we strip off any leading path components and the '.sh' suffix from the test script's pathname, and there are - two places where we construct the filename of test output files in - 't/test-results/'. The last patch in this series will add even more. + four places where we construct the name of the 't/test-results' + directory or the name of various test-specific files in there. The + last patch in this series will add even more. Factor these out into helper variables to avoid repeating ourselves. @@ -15,22 +16,23 @@ --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ - exit 1 - fi + esac + done +TEST_NAME="$(basename "$0" .sh)" -+TEST_RESULTS_BASE="$TEST_OUTPUT_DIRECTORY/test-results/$TEST_NAME" ++TEST_RESULTS_DIR="$TEST_OUTPUT_DIRECTORY/test-results" ++TEST_RESULTS_BASE="$TEST_RESULTS_DIR/$TEST_NAME" + # if --tee was passed, write the output not only to the terminal, but # additionally to the file test-results/$BASENAME.out, too. - case "$GIT_TEST_TEE_STARTED, $* " in + if test "$GIT_TEST_TEE_STARTED" = "done" @@ - # do not redirect again - ;; - *' --tee '*|*' --va'*|*' -V '*|*' --verbose-log '*) + elif test -n "$tee" || test -n "$verbose_log" || + test -n "$valgrind" || test -n "$valgrind_only" + then - mkdir -p "$TEST_OUTPUT_DIRECTORY/test-results" - BASE="$TEST_OUTPUT_DIRECTORY/test-results/$(basename "$0" .sh)" -+ mkdir -p "$(dirname "$TEST_RESULTS_BASE")" ++ mkdir -p "$TEST_RESULTS_DIR" # Make this filename available to the sub-process in case it is using # --verbose-log. @@ -48,8 +50,8 @@ + echo $? >"$TEST_RESULTS_BASE.exit") | tee -a "$GIT_TEST_TEE_OUTPUT_FILE" + test "$(cat "$TEST_RESULTS_BASE.exit")" = 0 exit - ;; - esac + fi + @@ if test -z "$HARNESS_ACTIVE" @@ -58,7 +60,7 @@ - mkdir -p "$test_results_dir" - base=${0##*/} - test_results_path="$test_results_dir/${base%.sh}.counts" -+ mkdir -p "$(dirname "$TEST_RESULTS_BASE")" ++ mkdir -p "$TEST_RESULTS_DIR" - cat >"$test_results_path" <<-EOF + cat >"$TEST_RESULTS_BASE.counts" <<-EOF -: ---------- > 4: f3941077e6 test-lib: set $TRASH_DIRECTORY earlier -: ---------- > 5: fefbca96ee test-lib: extract Bash version check for '-x' tracing 2: 9aec8662f9 ! 6: b4b6844a3e test-lib-functions: introduce the 'test_set_port' helper function @@ -27,7 +27,7 @@ containing the digits 8 or 9 will trigger an error. Remove all leading zeros from the test numbers to prevent this. - Note that the Perforce tests are unlike the other tests involving + Note that the 'git p4' tests are unlike the other tests involving daemons in that: - 'lib-git-p4.sh' doesn't use the test's number for unique port as @@ -35,7 +35,7 @@ - The port is not overridable via an environment variable. - With this patch even Perforce tests will use the test's number as + With this patch even 'git p4' tests will use the test's number as default port, and it will be overridable via the P4DPORT environment variable. @@ -161,7 +161,7 @@ + BUG "test_set_port requires a variable name" + fi + -+ eval port=\"\${$var}\" ++ eval port=\$$var + case "$port" in + "") + # No port is set in the given env var, use the test 3: a5aa71f20c < -: ---------- test-lib: add the '--stress' option to run a test repeatedly under load -: ---------- > 7: 8470b55f65 test-lib: add the '--stress' option to run a test repeatedly under load