{"thread":{"id":"40801","subject":"[PATCH v5 1/6] implement test_might_fail using a refactored test_must_fail","startedAt":"2015-11-15T13:08:34Z","lastAt":"2015-11-17T23:40:01Z","messageCount":19,"participants":["larsxschneider@gmail.com","Luke Diamand","Eric Sunshine","Lars Schneider"],"isPatch":true,"patchVersion":5,"patchTotal":6},"messages":[{"id":"273341","messageId":"1447592920-89228-1-git-send-email-larsxschneider@gmail.com","threadId":"40801","inReplyTo":null,"subject":"[PATCH v5 0/6] Add Travis CI support","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-15T13:08:34Z","receivedAt":"2015-11-15T13:08:34Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\ndiff to v4:\n* add Junio's \"test_must_fail ok=\" refactor that he sent as response\n to my patch. Did I attribute this to Junio in the right way by adding\n \"Signed-off-by: Junio C Hamano \" to the commit message?\n Please let me know how to handle these cases properly!\n* fix commit message wording according to Junio's suggestion\n* do not accept sigpipe failures in \"git cat-file\" (thanks Junio)\n* reorder patches (thanks Eric)\n* fix incorrect space indent (thanks Eric)\n* add a trap to git-p4 tests that ensure p4d is always killed\n\nYou can see the CI results for this patch applied on master here:\nhttps://travis-ci.org/larsxschneider/git/builds/91216501\n\nThanks,\nLars\n\nLars Schneider (6):\n implement test_might_fail using a refactored test_must_fail\n add \"ok=sigpipe\" to test_must_fail and use it to fix flaky tests\n git-p4: retry kill/cleanup operations in tests with timeout\n git-p4: add p4d timeout in tests\n git-p4: add trap to kill p4d on test exit\n Add Travis CI support\n\n .travis.yml | 131 ++++++++++++++++++++++++++++++++++++++++\n t/lib-git-p4.sh | 57 ++++++++++++++---\n t/t5504-fetch-receive-strict.sh | 3 +-\n t/t5516-fetch-push.sh | 6 +-\n t/test-lib-functions.sh | 39 +++++++-----\n 5 files changed, 207 insertions(+), 29 deletions(-)\n create mode 100644 .travis.yml\n\n--\n2.5.1\n"},{"id":"273339","messageId":"1447592920-89228-2-git-send-email-larsxschneider@gmail.com","threadId":"40801","inReplyTo":"1447592920-89228-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v5 1/6] implement test_might_fail using a refactored test_must_fail","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-15T13:08:35Z","receivedAt":"2015-11-15T13:08:35Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nAdd an (optional) first parameter \"ok=\" to test_must_fail\nand return success for \"\". Add \"success\" as\n\"\" and use it to implement \"test_might_fail\". This removes\nredundancies in test-lib-function.sh.\n\nSigned-off-by: Junio C Hamano \nSigned-off-by: Lars Schneider \n---\n t/test-lib-functions.sh | 35 ++++++++++++++++++++---------------\n 1 file changed, 20 insertions(+), 15 deletions(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 73e37a1..1e762da 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -582,18 +582,32 @@ test_line_count () {\n # the failure could be due to a segv. We want a controlled failure.\n \n test_must_fail () {\n+\tcase \"$1\" in\n+\tok=*)\n+\t\t_test_ok=${1#ok=}\n+\t\tshift\n+\t\t;;\n+\t*)\n+\t\t_test_ok=\n+\t\t;;\n+\tesac\n \t\"$@\"\n \texit_code=$?\n-\tif test $exit_code = 0; then\n+\tif ! case \",$_test_ok,\" in *,success,*) false;; esac &&\n+\t\ttest $exit_code = 0\n+\tthen\n \t\techo >&2 \"test_must_fail: command succeeded: $*\"\n-\t\treturn 1\n-\telif test $exit_code -gt 129 && test $exit_code -le 192; then\n+\t\treturn 0\n+\telif test $exit_code -gt 129 && test $exit_code -le 192\n+\tthen\n \t\techo >&2 \"test_must_fail: died by signal: $*\"\n \t\treturn 1\n-\telif test $exit_code = 127; then\n+\telif test $exit_code = 127\n+\tthen\n \t\techo >&2 \"test_must_fail: command not found: $*\"\n \t\treturn 1\n-\telif test $exit_code = 126; then\n+\telif test $exit_code = 126\n+\tthen\n \t\techo >&2 \"test_must_fail: valgrind error: $*\"\n \t\treturn 1\n \tfi\n@@ -612,16 +626,7 @@ test_must_fail () {\n # because we want to notice if it fails due to segv.\n \n test_might_fail () {\n-\t\"$@\"\n-\texit_code=$?\n-\tif test $exit_code -gt 129 && test $exit_code -le 192; then\n-\t\techo >&2 \"test_might_fail: died by signal: $*\"\n-\t\treturn 1\n-\telif test $exit_code = 127; then\n-\t\techo >&2 \"test_might_fail: command not found: $*\"\n-\t\treturn 1\n-\tfi\n-\treturn 0\n+\ttest_must_fail ok=success \"$@\"\n }\n \n # Similar to test_must_fail and test_might_fail, but check that a\n-- \n2.5.1\n"},{"id":"273343","messageId":"1447592920-89228-3-git-send-email-larsxschneider@gmail.com","threadId":"40801","inReplyTo":"1447592920-89228-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v5 2/6] add \"ok=sigpipe\" to test_must_fail and use it to fix flaky tests","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-15T13:08:36Z","receivedAt":"2015-11-15T13:08:36Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nt5516 \"75 - deny fetch unreachable SHA1, allowtipsha1inwant=true\" is\nflaky in the following case:\n1. remote upload-pack finds out \"not our ref\"\n2. remote sends a response and closes the pipe\n3. fetch-pack still tries to write commands to the remote upload-pack\n4. write call in wrapper.c dies with SIGPIPE\n\nt5504 \"9 - push with transfer.fsckobjects\" is flaky, too, and returns\nSIGPIPE once in a while. I had to remove the final \"To dst...\" output\ncheck because there is no output if the process dies with SIGPUPE.\n\nAccept such a death-with-sigpipe also as OK when we are expecting a\nfailure.\n\nSigned-off-by: Lars Schneider \n---\n t/t5504-fetch-receive-strict.sh | 3 +--\n t/t5516-fetch-push.sh | 6 +++---\n t/test-lib-functions.sh | 4 ++++\n 3 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh\nindex 44f3d5f..a9e382c 100755\n--- a/t/t5504-fetch-receive-strict.sh\n+++ b/t/t5504-fetch-receive-strict.sh\n@@ -111,8 +111,7 @@ test_expect_success 'push with transfer.fsckobjects' '\n \t\tcd dst &&\n \t\tgit config transfer.fsckobjects true\n \t) &&\n-\ttest_must_fail git push --porcelain dst master:refs/heads/test >act &&\n-\ttest_cmp exp act\n+\ttest_must_fail ok=sigpipe git push --porcelain dst master:refs/heads/test >act\n '\n \n cat >bogus-commit <<\\EOF\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex ec22c98..0a87e19 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1162,15 +1162,15 @@ do\n \t\tmk_empty shallow &&\n \t\t(\n \t\t\tcd shallow &&\n-\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_3 &&\n-\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_1 &&\n+\t\t\ttest_must_fail ok=sigpipe git fetch ../testrepo/.git $SHA1_3 &&\n+\t\t\ttest_must_fail ok=sigpipe git fetch ../testrepo/.git $SHA1_1 &&\n \t\t\tgit --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n \t\t\tgit fetch ../testrepo/.git $SHA1_1 &&\n \t\t\tgit cat-file commit $SHA1_1 &&\n \t\t\ttest_must_fail git cat-file commit $SHA1_2 &&\n \t\t\tgit fetch ../testrepo/.git $SHA1_2 &&\n \t\t\tgit cat-file commit $SHA1_2 &&\n-\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_3\n+\t\t\ttest_must_fail ok=sigpipe git fetch ../testrepo/.git $SHA1_3\n \t\t)\n \t'\n done\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 1e762da..1fdc58c 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -598,6 +598,10 @@ test_must_fail () {\n \tthen\n \t\techo >&2 \"test_must_fail: command succeeded: $*\"\n \t\treturn 0\n+\telif ! case \",$_test_ok,\" in *,sigpipe,*) false;; esac &&\n+\t\ttest $exit_code = 141\n+\tthen\n+\t\treturn 0\n \telif test $exit_code -gt 129 && test $exit_code -le 192\n \tthen\n \t\techo >&2 \"test_must_fail: died by signal: $*\"\n-- \n2.5.1\n"},{"id":"273340","messageId":"1447592920-89228-4-git-send-email-larsxschneider@gmail.com","threadId":"40801","inReplyTo":"1447592920-89228-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v5 3/6] git-p4: retry kill/cleanup operations in tests with timeout","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-15T13:08:37Z","receivedAt":"2015-11-15T13:08:37Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nIn rare cases kill/cleanup operations in tests fail. Retry these\noperations with a timeout to make the test less flaky.\n\nSigned-off-by: Lars Schneider \n---\n t/lib-git-p4.sh | 31 +++++++++++++++++++++++--------\n 1 file changed, 23 insertions(+), 8 deletions(-)\n\ndiff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\nindex 7548225..8d6b48f 100644\n--- a/t/lib-git-p4.sh\n+++ b/t/lib-git-p4.sh\n@@ -6,6 +6,10 @@\n # a subdirectory called \"$git\"\n TEST_NO_CREATE_REPO=NoThanks\n \n+# Some operations require multiple attempts to be successful. Define\n+# here the maximal retry timeout in seconds.\n+RETRY_TIMEOUT=60\n+\n . ./test-lib.sh\n \n if ! test_have_prereq PYTHON\n@@ -121,22 +125,33 @@ p4_add_user() {\n \tEOF\n }\n \n+retry_until_success() {\n+ timeout=$(($(date +%s) + $RETRY_TIMEOUT))\n+ until \"$@\" 2>/dev/null || test $(date +%s) -gt $timeout\n+ do :\n+ done\n+}\n+\n+retry_until_fail() {\n+ timeout=$(($(date +%s) + $RETRY_TIMEOUT))\n+ until ! \"$@\" 2>/dev/null || test $(date +%s) -gt $timeout\n+ do :\n+ done\n+}\n+\n kill_p4d() {\n \tpid=$(cat \"$pidfile\")\n-\t# it had better exist for the first kill\n-\tkill $pid &&\n-\tfor i in 1 2 3 4 5 ; do\n-\t\tkill $pid >/dev/null 2>&1 || break\n-\t\tsleep 1\n-\tdone &&\n+\tretry_until_fail kill $pid\n+\tretry_until_fail kill -9 $pid\n \t# complain if it would not die\n \ttest_must_fail kill $pid >/dev/null 2>&1 &&\n \trm -rf \"$db\" \"$cli\" \"$pidfile\"\n }\n \n cleanup_git() {\n-\trm -rf \"$git\" &&\n-\tmkdir \"$git\"\n+\tretry_until_success rm -r \"$git\"\n+\ttest_must_fail test -d \"$git\" &&\n+\tretry_until_success mkdir \"$git\"\n }\n \n marshal_dump() {\n-- \n2.5.1\n"},{"id":"273344","messageId":"1447592920-89228-5-git-send-email-larsxschneider@gmail.com","threadId":"40801","inReplyTo":"1447592920-89228-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v5 4/6] git-p4: add p4d timeout in tests","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-15T13:08:38Z","receivedAt":"2015-11-15T13:08:38Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nIn rare cases p4d seems to hang. This watchdog will kill the p4d\nprocess after 300s in any case. That means each individual git p4 test\nneeds to finish before 300s or it will fail.\n\nSigned-off-by: Lars Schneider \n---\n t/lib-git-p4.sh | 20 +++++++++++++++++++-\n 1 file changed, 19 insertions(+), 1 deletion(-)\n\ndiff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\nindex 8d6b48f..f2a009c 100644\n--- a/t/lib-git-p4.sh\n+++ b/t/lib-git-p4.sh\n@@ -10,6 +10,10 @@ TEST_NO_CREATE_REPO=NoThanks\n # here the maximal retry timeout in seconds.\n RETRY_TIMEOUT=60\n \n+# Sometimes p4d seems to hang. Terminate the p4d process automatically after\n+# the defined timeout in seconds.\n+P4D_TIMEOUT=300\n+\n . ./test-lib.sh\n \n if ! test_have_prereq PYTHON\n@@ -85,6 +89,19 @@ start_p4d() {\n \t# will be caught with the \"kill -0\" check below.\n \ti=${P4D_START_PATIENCE:-300}\n \tpid=$(cat \"$pidfile\")\n+\n+\ttimeout=$(($(date +%s) + $P4D_TIMEOUT))\n+\twhile true\n+\tdo\n+\t\tif test $(date +%s) -gt $timeout\n+\t\tthen\n+\t\t\tkill -9 $pid\n+\t\t\texit 1\n+\t\tfi\n+\t\tsleep 1\n+\tdone &\n+\twatchdog_pid=$!\n+\n \tready=\n \twhile test $i -gt 0\n \tdo\n@@ -145,7 +162,8 @@ kill_p4d() {\n \tretry_until_fail kill -9 $pid\n \t# complain if it would not die\n \ttest_must_fail kill $pid >/dev/null 2>&1 &&\n-\trm -rf \"$db\" \"$cli\" \"$pidfile\"\n+\trm -rf \"$db\" \"$cli\" \"$pidfile\" &&\n+\tretry_until_fail kill -9 $watchdog_pid\n }\n \n cleanup_git() {\n-- \n2.5.1\n"},{"id":"273345","messageId":"1447592920-89228-6-git-send-email-larsxschneider@gmail.com","threadId":"40801","inReplyTo":"1447592920-89228-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v5 5/6] git-p4: add trap to kill p4d on test exit","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-15T13:08:39Z","receivedAt":"2015-11-15T13:08:39Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nSometimes the \"prove\" test runner hangs on test exit because p4d is\nstill running. Add a trap to always kill \"p4d\" on test exit.\n\nYou can reproduce the problem by commenting \"P4D_TIMEOUT\" in\n\"lib-git-p4.sh\" and running \"prove ./t9800-git-p4-basic.sh\".\n---\n t/lib-git-p4.sh | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\nindex f2a009c..f9c68d4 100644\n--- a/t/lib-git-p4.sh\n+++ b/t/lib-git-p4.sh\n@@ -65,6 +65,12 @@ cli=\"$TRASH_DIRECTORY/cli\"\n git=\"$TRASH_DIRECTORY/git\"\n pidfile=\"$TRASH_DIRECTORY/p4d.pid\"\n \n+# Sometimes \"prove\" seems to hang on exit because p4d is still running\n+cleanup() {\n+\tkill -9 $(cat \"$pidfile\") 2>/dev/null && exit 255\n+}\n+trap cleanup EXIT\n+\n # git p4 submit generates a temp file, which will\n # not get cleaned up if the submission fails. Don't\n # clutter up /tmp on the test machine.\n-- \n2.5.1\n"},{"id":"273342","messageId":"1447592920-89228-7-git-send-email-larsxschneider@gmail.com","threadId":"40801","inReplyTo":"1447592920-89228-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v5 6/6] Add Travis CI support","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-15T13:08:40Z","receivedAt":"2015-11-15T13:08:40Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nThe tests are currently executed on \"Ubuntu 12.04 LTS Server Edition\n64 bit\" and on \"OS X Mavericks\" using gcc and clang.\n\nPerforce and Git-LFS are installed and therefore available for the\nrespective tests.\n\nSigned-off-by: Lars Schneider \n---\n .travis.yml | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 131 insertions(+)\n create mode 100644 .travis.yml\n\ndiff --git a/.travis.yml b/.travis.yml\nnew file mode 100644\nindex 0000000..61c70fa\n--- /dev/null\n+++ b/.travis.yml\n@@ -0,0 +1,131 @@\n+language: c\n+\n+os:\n+ - linux\n+ - osx\n+\n+compiler:\n+ - clang\n+ - gcc\n+\n+addons:\n+ apt:\n+ packages:\n+ - language-pack-is\n+\n+env:\n+ global:\n+ - P4_VERSION=\"15.1\"\n+ - GIT_LFS_VERSION=\"1.0.2\"\n+ - DEFAULT_TEST_TARGET=prove\n+ - GIT_PROVE_OPTS=\"--timer --jobs 3\"\n+ - GIT_TEST_OPTS=\"--verbose --tee\"\n+ - GETTEXT_ISO_LOCALE=YesPlease\n+ - GETTEXT_LOCALE=YesPlease\n+ # - GETTEXT_POISON=YesPlease\n+ - GIT_TEST_CHAIN_LINT=YesPlease\n+ - GIT_TEST_CLONE_2GB=YesPlease\n+ # - GIT_TEST_LONG=YesPlease\n+ matrix:\n+ -\n+ # NO_ICONV=YesPlease\n+ - >\n+ NO_CURL=YesPlease\n+ NO_D_INO_IN_DIRENT=YesPlease\n+ NO_DEFLATE_BOUND=YesPlease\n+ NO_EXPAT=YesPlease\n+ NO_GECOS_IN_PWENT=YesPlease\n+ NO_GETTEXT=YesPlease\n+ NO_HMAC_CTX_CLEANUP=YesPlease\n+ NO_HSTRERROR=YesPlease\n+ NO_INET_NTOP=YesPlease\n+ NO_INET_PTON=YesPlease\n+ NO_INITGROUPS=YesPlease\n+ NO_INTTYPES_H=YesPlease\n+ NO_IPV6=YesPlease\n+ NO_IPV6=YesPlease\n+ NO_LIBGEN_H=YesPlease\n+ NO_MEMMEM=YesPlease\n+ NO_MKDTEMP=YesPlease\n+ NO_MKSTEMPS=YesPlease\n+ NO_MMAP=YesPlease\n+ NO_NSEC=YesPlease\n+ NO_OPENSSL=YesPlease\n+ NO_PERL=YesPlease\n+ NO_PTHREADS=YesPlease\n+ NO_REGEX=YesPlease\n+ NO_SETENV=YesPlease\n+ NO_SETITIMER=YesPlease\n+ NO_SOCKADDR_STORAGE=YesPlease\n+ NO_STRCASESTR=YesPlease\n+ NO_STRLCPY=YesPlease\n+ NO_STRTOUMAX=YesPlease\n+ NO_STRUCT_ITIMERVAL=YesPlease\n+ NO_SYMLINK_HEAD=YesPlease\n+ NO_SYS_POLL_H=YesPlease\n+ NO_SYS_SELECT_H=YesPlease\n+ NO_UINTMAX_T=YesPlease\n+ NO_UNSETENV=YesPlease\n+\n+before_install:\n+ - >\n+ case \"${TRAVIS_OS_NAME:-linux}\" in\n+ linux)\n+ mkdir --parents custom/p4\n+ pushd custom/p4\n+ wget --quiet http://filehost.perforce.com/perforce/r$P4_VERSION/bin.linux26x86_64/p4d\n+ wget --quiet http://filehost.perforce.com/perforce/r$P4_VERSION/bin.linux26x86_64/p4\n+ chmod u+x p4d\n+ chmod u+x p4\n+ export PATH=\"$(pwd):$PATH\"\n+ popd\n+ mkdir --parents custom/git-lfs\n+ pushd custom/git-lfs\n+ wget --quiet https://github.com/github/git-lfs/releases/download/v$GIT_LFS_VERSION/git-lfs-linux-amd64-$GIT_LFS_VERSION.tar.gz\n+ tar --extract --gunzip --file \"git-lfs-linux-amd64-$GIT_LFS_VERSION.tar.gz\"\n+ cp git-lfs-$GIT_LFS_VERSION/git-lfs .\n+ export PATH=\"$(pwd):$PATH\"\n+ popd\n+ ;;\n+ osx)\n+ brew_force_set_latest_binary_hash () {\n+ FORMULA=$1\n+ SHA=$(brew fetch --force $FORMULA 2>&1 | grep ^SHA256: | cut -d ' ' -f 2)\n+ sed -E -i.bak \"s/sha256 \\\"[0-9a-f]{64}\\\"/sha256 \\\"$SHA\\\"/g\" \\\n+ /usr/local/Library/Taps/homebrew/homebrew-binary/$FORMULA.rb\n+ }\n+ brew update --quiet\n+ brew tap homebrew/binary --quiet\n+ brew_force_set_latest_binary_hash perforce\n+ brew_force_set_latest_binary_hash perforce-server\n+ brew install git-lfs perforce-server perforce\n+ ;;\n+ esac;\n+ echo \"$(tput setaf 6)Perfoce Server Version$(tput sgr0)\";\n+ p4d -V | grep Rev.;\n+ echo \"$(tput setaf 6)Perfoce Client Version$(tput sgr0)\";\n+ p4 -V | grep Rev.;\n+ echo \"$(tput setaf 6)Git-LFS Version$(tput sgr0)\";\n+ git-lfs version;\n+\n+before_script: make configure && ./configure && make --jobs=2\n+\n+script: make --quiet test\n+\n+after_failure:\n+ - >\n+ : '<-- Click here to see detailed test output! ';\n+ for TEST_EXIT in t/test-results/*.exit;\n+ do\n+ if [ \"$(cat \"$TEST_EXIT\")\" != \"0\" ];\n+ then\n+ TEST_OUT=\"${TEST_EXIT%exit}out\";\n+ echo \"------------------------------------------------------------------------\";\n+ echo \"$(tput setaf 1)${TEST_OUT}...$(tput sgr0)\";\n+ echo \"------------------------------------------------------------------------\";\n+ cat \"${TEST_OUT}\";\n+ fi;\n+ done;\n+\n+notifications:\n+ email: false\n-- \n2.5.1\n"},{"id":"273356","messageId":"5649958F.2010407@diamand.org","threadId":"40801","inReplyTo":"1447592920-89228-4-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v5 3/6] git-p4: retry kill/cleanup operations in tests with timeout","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-11-16T08:36:31Z","receivedAt":"2015-11-16T08:36:31Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 15/11/15 13:08, larsxschneider@gmail.com wrote:\n> From: Lars Schneider \n>\n> In rare cases kill/cleanup operations in tests fail. Retry these\n> operations with a timeout to make the test less flaky.\n\nShould there be a sleep in that retry_until_success loop so that it \ndoesn't spin sending signals to p4d?\n\nDo we need to worry about the time offset being updated (e.g. NTP) while \nthis is running?\n\n>\n> Signed-off-by: Lars Schneider \n> ---\n> t/lib-git-p4.sh | 31 +++++++++++++++++++++++--------\n> 1 file changed, 23 insertions(+), 8 deletions(-)\n>\n> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n> index 7548225..8d6b48f 100644\n> --- a/t/lib-git-p4.sh\n> +++ b/t/lib-git-p4.sh\n> @@ -6,6 +6,10 @@\n> # a subdirectory called \"$git\"\n> TEST_NO_CREATE_REPO=NoThanks\n>\n> +# Some operations require multiple attempts to be successful. Define\n> +# here the maximal retry timeout in seconds.\n> +RETRY_TIMEOUT=60\n> +\n> . ./test-lib.sh\n>\n> if ! test_have_prereq PYTHON\n> @@ -121,22 +125,33 @@ p4_add_user() {\n> \tEOF\n> }\n>\n> +retry_until_success() {\n> + timeout=$(($(date +%s) + $RETRY_TIMEOUT))\n> + until \"$@\" 2>/dev/null || test $(date +%s) -gt $timeout\n> + do :\n\n sleep here?\n\n> + done\n> +}\n> +\n> +retry_until_fail() {\n> + timeout=$(($(date +%s) + $RETRY_TIMEOUT))\n> + until ! \"$@\" 2>/dev/null || test $(date +%s) -gt $timeout\n> + do :\n\n sleep here?\n\n> + done\n> +}\n> +\n> kill_p4d() {\n> \tpid=$(cat \"$pidfile\")\n> -\t# it had better exist for the first kill\n> -\tkill $pid &&\n> -\tfor i in 1 2 3 4 5 ; do\n> -\t\tkill $pid >/dev/null 2>&1 || break\n> -\t\tsleep 1\n> -\tdone &&\n> +\tretry_until_fail kill $pid\n> +\tretry_until_fail kill -9 $pid\n> \t# complain if it would not die\n> \ttest_must_fail kill $pid >/dev/null 2>&1 &&\n> \trm -rf \"$db\" \"$cli\" \"$pidfile\"\n> }\n>\n> cleanup_git() {\n> -\trm -rf \"$git\" &&\n> -\tmkdir \"$git\"\n> +\tretry_until_success rm -r \"$git\"\n> +\ttest_must_fail test -d \"$git\" &&\n> +\tretry_until_success mkdir \"$git\"\n> }\n>\n> marshal_dump() {\n>\n"},{"id":"273357","messageId":"56499659.9070206@diamand.org","threadId":"40801","inReplyTo":"1447592920-89228-5-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v5 4/6] git-p4: add p4d timeout in tests","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-11-16T08:39:53Z","receivedAt":"2015-11-16T08:39:53Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 15/11/15 13:08, larsxschneider@gmail.com wrote:\n> From: Lars Schneider \n>\n> In rare cases p4d seems to hang. This watchdog will kill the p4d\n> process after 300s in any case. That means each individual git p4 test\n> needs to finish before 300s or it will fail.\n\nLooks good to me.\n\n\n>\n> Signed-off-by: Lars Schneider \n> ---\n> t/lib-git-p4.sh | 20 +++++++++++++++++++-\n> 1 file changed, 19 insertions(+), 1 deletion(-)\n>\n> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n> index 8d6b48f..f2a009c 100644\n> --- a/t/lib-git-p4.sh\n> +++ b/t/lib-git-p4.sh\n> @@ -10,6 +10,10 @@ TEST_NO_CREATE_REPO=NoThanks\n> # here the maximal retry timeout in seconds.\n> RETRY_TIMEOUT=60\n>\n> +# Sometimes p4d seems to hang. Terminate the p4d process automatically after\n> +# the defined timeout in seconds.\n> +P4D_TIMEOUT=300\n> +\n> . ./test-lib.sh\n>\n> if ! test_have_prereq PYTHON\n> @@ -85,6 +89,19 @@ start_p4d() {\n> \t# will be caught with the \"kill -0\" check below.\n> \ti=${P4D_START_PATIENCE:-300}\n> \tpid=$(cat \"$pidfile\")\n> +\n> +\ttimeout=$(($(date +%s) + $P4D_TIMEOUT))\n> +\twhile true\n> +\tdo\n> +\t\tif test $(date +%s) -gt $timeout\n> +\t\tthen\n> +\t\t\tkill -9 $pid\n> +\t\t\texit 1\n> +\t\tfi\n> +\t\tsleep 1\n> +\tdone &\n> +\twatchdog_pid=$!\n> +\n> \tready=\n> \twhile test $i -gt 0\n> \tdo\n> @@ -145,7 +162,8 @@ kill_p4d() {\n> \tretry_until_fail kill -9 $pid\n> \t# complain if it would not die\n> \ttest_must_fail kill $pid >/dev/null 2>&1 &&\n> -\trm -rf \"$db\" \"$cli\" \"$pidfile\"\n> +\trm -rf \"$db\" \"$cli\" \"$pidfile\" &&\n> +\tretry_until_fail kill -9 $watchdog_pid\n> }\n>\n> cleanup_git() {\n>\n"},{"id":"273358","messageId":"5649974A.1010903@diamand.org","threadId":"40801","inReplyTo":"1447592920-89228-6-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v5 5/6] git-p4: add trap to kill p4d on test exit","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-11-16T08:43:54Z","receivedAt":"2015-11-16T08:43:54Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 15/11/15 13:08, larsxschneider@gmail.com wrote:\n> From: Lars Schneider \n>\n> Sometimes the \"prove\" test runner hangs on test exit because p4d is\n> still running. Add a trap to always kill \"p4d\" on test exit.\n\nWith this change, I've started seeing this when running the tests:\n\ncat: /home/lgd/git/git/t/trash \ndirectory.t9819-git-p4-case-folding/p4d.pid: No such file or directory\n\nProbably just needs the obvious \"test -f\" adding.\n\nOther than, all looks good to me. Particularly nice that I can now do:\n\n$ make T=t98* -j10\n\nand it actually works!\n\n\n\n\n\n>\n> You can reproduce the problem by commenting \"P4D_TIMEOUT\" in\n> \"lib-git-p4.sh\" and running \"prove ./t9800-git-p4-basic.sh\".\n> ---\n> t/lib-git-p4.sh | 6 ++++++\n> 1 file changed, 6 insertions(+)\n>\n> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n> index f2a009c..f9c68d4 100644\n> --- a/t/lib-git-p4.sh\n> +++ b/t/lib-git-p4.sh\n> @@ -65,6 +65,12 @@ cli=\"$TRASH_DIRECTORY/cli\"\n> git=\"$TRASH_DIRECTORY/git\"\n> pidfile=\"$TRASH_DIRECTORY/p4d.pid\"\n>\n> +# Sometimes \"prove\" seems to hang on exit because p4d is still running\n> +cleanup() {\n> +\tkill -9 $(cat \"$pidfile\") 2>/dev/null && exit 255\n> +}\n> +trap cleanup EXIT\n> +\n> # git p4 submit generates a temp file, which will\n> # not get cleaned up if the submission fails. Don't\n> # clutter up /tmp on the test machine.\n>\n"},{"id":"273381","messageId":"CAPig+cQkNB3VzbC_R_T=ppkgvmTbOWnK22hFxyMzJH1DZ_iVhQ@mail.gmail.com","threadId":"40801","inReplyTo":"1447592920-89228-4-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v5 3/6] git-p4: retry kill/cleanup operations in tests with timeout","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-11-16T21:14:06Z","receivedAt":"2015-11-16T21:14:06Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Nov 15, 2015 at 8:08 AM, wrote:\n> From: Lars Schneider \n>\n> In rare cases kill/cleanup operations in tests fail. Retry these\n> operations with a timeout to make the test less flaky.\n>\n> Signed-off-by: Lars Schneider \n> ---\n> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n> @@ -121,22 +125,33 @@ p4_add_user() {\n> EOF\n> }\n>\n> +retry_until_success() {\n> + timeout=$(($(date +%s) + $RETRY_TIMEOUT))\n\nThere was some discussion previously[1] about detecting dynamically\nwhether 'date +%s' was supported. Was this something that you intended\nto do, or did you decide against it since p4 is not supported on such\nplatforms?\n\nSame question also applies to patch 4/6.\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/280978/match=lazy+prerequisite\n\n> + until \"$@\" 2>/dev/null || test $(date +%s) -gt $timeout\n> + do :\n> + done\n> +}\n> +\n> +retry_until_fail() {\n> + timeout=$(($(date +%s) + $RETRY_TIMEOUT))\n> + until ! \"$@\" 2>/dev/null || test $(date +%s) -gt $timeout\n> + do :\n> + done\n> +}\n"},{"id":"273411","messageId":"90549E9A-205A-4848-B794-23E44F325F1E@gmail.com","threadId":"40801","inReplyTo":"5649958F.2010407@diamand.org","subject":"Re: [PATCH v5 3/6] git-p4: retry kill/cleanup operations in tests with timeout","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-17T08:22:03Z","receivedAt":"2015-11-17T08:22:03Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 16 Nov 2015, at 09:36, Luke Diamand wrote:\n\n> On 15/11/15 13:08, larsxschneider@gmail.com wrote:\n>> From: Lars Schneider \n>> \n>> In rare cases kill/cleanup operations in tests fail. Retry these\n>> operations with a timeout to make the test less flaky.\n> \n> Should there be a sleep in that retry_until_success loop so that it doesn't spin sending signals to p4d?\nAgreed. I will add a sleep in the next roll!\n\n\n> \n> Do we need to worry about the time offset being updated (e.g. NTP) while this is running?\nInteresting question! I would consider this an edge case but I can see how it could happen.\nDo you see a way to handle that in an easy way?\n\nThanks,\nLars\n\n> \n>> \n>> Signed-off-by: Lars Schneider \n>> ---\n>> t/lib-git-p4.sh | 31 +++++++++++++++++++++++--------\n>> 1 file changed, 23 insertions(+), 8 deletions(-)\n>> \n>> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n>> index 7548225..8d6b48f 100644\n>> --- a/t/lib-git-p4.sh\n>> +++ b/t/lib-git-p4.sh\n>> @@ -6,6 +6,10 @@\n>> # a subdirectory called \"$git\"\n>> TEST_NO_CREATE_REPO=NoThanks\n>> \n>> +# Some operations require multiple attempts to be successful. Define\n>> +# here the maximal retry timeout in seconds.\n>> +RETRY_TIMEOUT=60\n>> +\n>> . ./test-lib.sh\n>> \n>> if ! test_have_prereq PYTHON\n>> @@ -121,22 +125,33 @@ p4_add_user() {\n>> \tEOF\n>> }\n>> \n>> +retry_until_success() {\n>> + timeout=$(($(date +%s) + $RETRY_TIMEOUT))\n>> + until \"$@\" 2>/dev/null || test $(date +%s) -gt $timeout\n>> + do :\n> \n> sleep here?\n> \n>> + done\n>> +}\n>> +\n>> +retry_until_fail() {\n>> + timeout=$(($(date +%s) + $RETRY_TIMEOUT))\n>> + until ! \"$@\" 2>/dev/null || test $(date +%s) -gt $timeout\n>> + do :\n> \n> sleep here?\n> \n>> + done\n>> +}\n>> +\n>> kill_p4d() {\n>> \tpid=$(cat \"$pidfile\")\n>> -\t# it had better exist for the first kill\n>> -\tkill $pid &&\n>> -\tfor i in 1 2 3 4 5 ; do\n>> -\t\tkill $pid >/dev/null 2>&1 || break\n>> -\t\tsleep 1\n>> -\tdone &&\n>> +\tretry_until_fail kill $pid\n>> +\tretry_until_fail kill -9 $pid\n>> \t# complain if it would not die\n>> \ttest_must_fail kill $pid >/dev/null 2>&1 &&\n>> \trm -rf \"$db\" \"$cli\" \"$pidfile\"\n>> }\n>> \n>> cleanup_git() {\n>> -\trm -rf \"$git\" &&\n>> -\tmkdir \"$git\"\n>> +\tretry_until_success rm -r \"$git\"\n>> +\ttest_must_fail test -d \"$git\" &&\n>> +\tretry_until_success mkdir \"$git\"\n>> }\n>> \n>> marshal_dump() {\n"},{"id":"273413","messageId":"52A9F66D-12C4-4BB5-A2D4-C476A1E12DC5@gmail.com","threadId":"40801","inReplyTo":"CAPig+cQkNB3VzbC_R_T=ppkgvmTbOWnK22hFxyMzJH1DZ_iVhQ@mail.gmail.com","subject":"Re: [PATCH v5 3/6] git-p4: retry kill/cleanup operations in tests with timeout","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-17T08:28:07Z","receivedAt":"2015-11-17T08:28:07Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 16 Nov 2015, at 22:14, Eric Sunshine wrote:\n\n> On Sun, Nov 15, 2015 at 8:08 AM, wrote:\n>> From: Lars Schneider \n>> \n>> In rare cases kill/cleanup operations in tests fail. Retry these\n>> operations with a timeout to make the test less flaky.\n>> \n>> Signed-off-by: Lars Schneider \n>> ---\n>> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n>> @@ -121,22 +125,33 @@ p4_add_user() {\n>> EOF\n>> }\n>> \n>> +retry_until_success() {\n>> + timeout=$(($(date +%s) + $RETRY_TIMEOUT))\n> \n> There was some discussion previously[1] about detecting dynamically\n> whether 'date +%s' was supported. Was this something that you intended\n> to do, or did you decide against it since p4 is not supported on such\n> platforms?\n> \n> Same question also applies to patch 4/6.\n\nWhile implementing it I thought more about it. P4D is only supported on platforms that support the date function. That means these tests will only run on platforms that support the date function. Consequently I wondered if this would justify the slightly more complicated code. However, if you think this change would help the patch to get accepted then I will add it.\n\nThanks,\nLars\n\n\n> \n> [1]: http://article.gmane.org/gmane.comp.version-control.git/280978/match=lazy+prerequisite\n> \n>> + until \"$@\" 2>/dev/null || test $(date +%s) -gt $timeout\n>> + do :\n>> + done\n>> +}\n>> +\n>> +retry_until_fail() {\n>> + timeout=$(($(date +%s) + $RETRY_TIMEOUT))\n>> + until ! \"$@\" 2>/dev/null || test $(date +%s) -gt $timeout\n>> + do :\n>> + done\n>> +}\n"},{"id":"273414","messageId":"8C9A93DB-040E-49FE-B6F7-8373D8CE7601@gmail.com","threadId":"40801","inReplyTo":"5649974A.1010903@diamand.org","subject":"Re: [PATCH v5 5/6] git-p4: add trap to kill p4d on test exit","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-17T08:29:49Z","receivedAt":"2015-11-17T08:29:49Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 16 Nov 2015, at 09:43, Luke Diamand wrote:\n\n> On 15/11/15 13:08, larsxschneider@gmail.com wrote:\n>> From: Lars Schneider \n>> \n>> Sometimes the \"prove\" test runner hangs on test exit because p4d is\n>> still running. Add a trap to always kill \"p4d\" on test exit.\n> \n> With this change, I've started seeing this when running the tests:\n> \n> cat: /home/lgd/git/git/t/trash directory.t9819-git-p4-case-folding/p4d.pid: No such file or directory\n> \n> Probably just needs the obvious \"test -f\" adding.\n> \n> Other than, all looks good to me. Particularly nice that I can now do:\n> \n> $ make T=t98* -j10\n> \n> and it actually works!\n\nGreat! I can see where the cat error comes from. I will add the \"test -f\" condition in the next roll.\n\nThanks,\nLars\n\n> \n> \n> \n> \n> \n>> \n>> You can reproduce the problem by commenting \"P4D_TIMEOUT\" in\n>> \"lib-git-p4.sh\" and running \"prove ./t9800-git-p4-basic.sh\".\n>> ---\n>> t/lib-git-p4.sh | 6 ++++++\n>> 1 file changed, 6 insertions(+)\n>> \n>> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n>> index f2a009c..f9c68d4 100644\n>> --- a/t/lib-git-p4.sh\n>> +++ b/t/lib-git-p4.sh\n>> @@ -65,6 +65,12 @@ cli=\"$TRASH_DIRECTORY/cli\"\n>> git=\"$TRASH_DIRECTORY/git\"\n>> pidfile=\"$TRASH_DIRECTORY/p4d.pid\"\n>> \n>> +# Sometimes \"prove\" seems to hang on exit because p4d is still running\n>> +cleanup() {\n>> +\tkill -9 $(cat \"$pidfile\") 2>/dev/null && exit 255\n>> +}\n>> +trap cleanup EXIT\n>> +\n>> # git p4 submit generates a temp file, which will\n>> # not get cleaned up if the submission fails. Don't\n>> # clutter up /tmp on the test machine.\n>> \n> \n"},{"id":"273416","messageId":"CAPig+cR8igO1bWNgma_yZqrcru_jnKchpRH+i2_AaNj8Atcb9g@mail.gmail.com","threadId":"40801","inReplyTo":"52A9F66D-12C4-4BB5-A2D4-C476A1E12DC5@gmail.com","subject":"Re: [PATCH v5 3/6] git-p4: retry kill/cleanup operations in tests with timeout","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-11-17T08:35:14Z","receivedAt":"2015-11-17T08:35:14Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Nov 17, 2015 at 3:28 AM, Lars Schneider\n wrote:\n> On 16 Nov 2015, at 22:14, Eric Sunshine wrote:\n>> On Sun, Nov 15, 2015 at 8:08 AM, wrote:\n>>> From: Lars Schneider \n>>>\n>>> In rare cases kill/cleanup operations in tests fail. Retry these\n>>> operations with a timeout to make the test less flaky.\n>>>\n>>> Signed-off-by: Lars Schneider \n>>> ---\n>>> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n>>> @@ -121,22 +125,33 @@ p4_add_user() {\n>>> EOF\n>>> }\n>>>\n>>> +retry_until_success() {\n>>> + timeout=$(($(date +%s) + $RETRY_TIMEOUT))\n>>\n>> There was some discussion previously[1] about detecting dynamically\n>> whether 'date +%s' was supported. Was this something that you intended\n>> to do, or did you decide against it since p4 is not supported on such\n>> platforms?\n>>\n>> Same question also applies to patch 4/6.\n>\n> While implementing it I thought more about it. P4D is only\n> supported on platforms that support the date function. That means\n> these tests will only run on platforms that support the date\n> function. Consequently I wondered if this would justify the\n> slightly more complicated code. However, if you think this change\n> would help the patch to get accepted then I will add it.\n\nI don't feel strongly about it, and it's not my call anyhow. Opinions\nof Junio, Peff (as interim maintainer), and Luke weigh much more\nheavily than my own. Punting on dynamic detection of \"date +%s\" may be\nperfectly acceptable with the attitude that it can be implemented\nlater if someone runs across a case where it's actually needed.\n"},{"id":"273417","messageId":"564AF48C.9040406@diamand.org","threadId":"40801","inReplyTo":"CAPig+cR8igO1bWNgma_yZqrcru_jnKchpRH+i2_AaNj8Atcb9g@mail.gmail.com","subject":"Re: [PATCH v5 3/6] git-p4: retry kill/cleanup operations in tests with timeout","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-11-17T09:34:04Z","receivedAt":"2015-11-17T09:34:04Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":">>\n>> While implementing it I thought more about it. P4D is only\n>> supported on platforms that support the date function. That means\n>> these tests will only run on platforms that support the date\n>> function. Consequently I wondered if this would justify the\n>> slightly more complicated code. However, if you think this change\n>> would help the patch to get accepted then I will add it.\n>\n> I don't feel strongly about it, and it's not my call anyhow. Opinions\n> of Junio, Peff (as interim maintainer), and Luke weigh much more\n> heavily than my own. Punting on dynamic detection of \"date +%s\" may be\n> perfectly acceptable with the attitude that it can be implemented\n> later if someone runs across a case where it's actually needed.\n\nWhich other platforms are we talking about here?\n\nhttps://www.perforce.com/downloads/helix\n\n From there, you can get Solaris10, HP-UX, AIX and various flavours of \nBSD. Solaris supports \"date +%s\".\n\nHP-UX and AIX, I really don't know.\n\nWindows? I assume 'date +%s' will work for people using mingw.\n\nIs it possible to get the time in seconds by doing something like this:\n\ntime_in_seconds() {\n\tpython -c 'import time; print time.time()'\n}\n\nLuke\n"},{"id":"273418","messageId":"564AF59F.2070601@diamand.org","threadId":"40801","inReplyTo":"90549E9A-205A-4848-B794-23E44F325F1E@gmail.com","subject":"Re: [PATCH v5 3/6] git-p4: retry kill/cleanup operations in tests with timeout","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-11-17T09:38:39Z","receivedAt":"2015-11-17T09:38:39Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 17/11/15 08:22, Lars Schneider wrote:\n>\n> On 16 Nov 2015, at 09:36, Luke Diamand wrote:\n>\n>> On 15/11/15 13:08, larsxschneider@gmail.com wrote:\n>>> From: Lars Schneider \n>>>\n>>> In rare cases kill/cleanup operations in tests fail. Retry these\n>>> operations with a timeout to make the test less flaky.\n>>\n>> Should there be a sleep in that retry_until_success loop so that it doesn't spin sending signals to p4d?\n> Agreed. I will add a sleep in the next roll!\n>\n>\n>>\n>> Do we need to worry about the time offset being updated (e.g. NTP) while this is running?\n> Interesting question! I would consider this an edge case but I can see how it could happen.\n> Do you see a way to handle that in an easy way?\n>\n\nYou want to somehow call clock_gettime(CLOCK_MONOTONIC). That's not in \npython until 3.3. Writing a C program seems like overkill but could be a \nsolution if this becomes a problem.\n"},{"id":"273438","messageId":"CAPig+cT2aBrL6Qh3W3d-ODjj7S0GORkTZMyQ6PBfHCLVqeKBvQ@mail.gmail.com","threadId":"40801","inReplyTo":"564AF48C.9040406@diamand.org","subject":"Re: [PATCH v5 3/6] git-p4: retry kill/cleanup operations in tests with timeout","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-11-17T17:44:30Z","receivedAt":"2015-11-17T17:44:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Nov 17, 2015 at 4:34 AM, Luke Diamand wrote:\n>>> While implementing it I thought more about it. P4D is only\n>>> supported on platforms that support the date function. That means\n>>> these tests will only run on platforms that support the date\n>>> function. Consequently I wondered if this would justify the\n>>> slightly more complicated code. However, if you think this change\n>>> would help the patch to get accepted then I will add it.\n>>\n>> I don't feel strongly about it, and it's not my call anyhow. Opinions\n>> of Junio, Peff (as interim maintainer), and Luke weigh much more\n>> heavily than my own. Punting on dynamic detection of \"date +%s\" may be\n>> perfectly acceptable with the attitude that it can be implemented\n>> later if someone runs across a case where it's actually needed.\n>\n> Which other platforms are we talking about here?\n>\n> https://www.perforce.com/downloads/helix\n>\n> From there, you can get Solaris10, HP-UX, AIX and various flavours of BSD.\n> Solaris supports \"date +%s\".\n\nThe question about \"date +%s\" portability arose with a suggestion to\nemploy it in git-filter-branch[1]. Apparently, the concern about\nSolaris was raised in response to a Stack Overflow question[2] which\nclaimed that +%s was not supported by that OS. Unfortunately, however,\n[2] did not indicate to which (older?) versions of Solaris that\nshortcoming applied. If Solaris 10 does support +%s, and the Perforce\nproduct is only available for Solaris 10, then perhaps concern about\n+%s a non-issue(?).\n\n[1]: http://git.661346.n2.nabble.com/FEATURE-REQUEST-Filter-branch-extend-progress-with-a-simple-estimated-time-remaning-td7638200.html#a7638504\n[2]: http://stackoverflow.com/questions/2445198/get-seconds-since-epoch-in-any-posix-compliant-shell\n"},{"id":"273456","messageId":"CAE5ih78ma-71xVgA52LDiWNyPq+tCmXb3GgocO=dF81=yZXuWw@mail.gmail.com","threadId":"40801","inReplyTo":"CAPig+cT2aBrL6Qh3W3d-ODjj7S0GORkTZMyQ6PBfHCLVqeKBvQ@mail.gmail.com","subject":"Re: [PATCH v5 3/6] git-p4: retry kill/cleanup operations in tests with timeout","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-11-17T23:40:01Z","receivedAt":"2015-11-17T23:40:01Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":">>\n>> Which other platforms are we talking about here?\n>>\n>> https://www.perforce.com/downloads/helix\n>>\n>> From there, you can get Solaris10, HP-UX, AIX and various flavours of BSD.\n>> Solaris supports \"date +%s\".\n>\n> The question about \"date +%s\" portability arose with a suggestion to\n> employ it in git-filter-branch[1]. Apparently, the concern about\n> Solaris was raised in response to a Stack Overflow question[2] which\n> claimed that +%s was not supported by that OS. Unfortunately, however,\n> [2] did not indicate to which (older?) versions of Solaris that\n> shortcoming applied. If Solaris 10 does support +%s, and the Perforce\n> product is only available for Solaris 10, then perhaps concern about\n> +%s a non-issue(?).\n>\n> [1]: http://git.661346.n2.nabble.com/FEATURE-REQUEST-Filter-branch-extend-progress-with-a-simple-estimated-time-remaning-td7638200.html#a7638504\n> [2]: http://stackoverflow.com/questions/2445198/get-seconds-since-epoch-in-any-posix-compliant-shell\n\nRereading the man page more carefully, you're right, Solaris 10\ndoesn't do \"%s\". Using python to get the seconds should work though.\n"}]}