{"thread":{"id":"40738","subject":"[PATCH v4 0/4] Add Travis CI support","startedAt":"2015-11-06T08:58:39Z","lastAt":"2015-11-06T18:49:15Z","messageCount":20,"participants":["larsxschneider@gmail.com","Eric Sunshine","Lars Schneider","Sebastian Schuberth","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":4},"messages":[{"id":"272996","messageId":"1446800323-2914-1-git-send-email-larsxschneider@gmail.com","threadId":"40738","inReplyTo":null,"subject":"[PATCH v4 0/4] Add Travis CI support","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-06T08:58:39Z","receivedAt":"2015-11-06T08:58:39Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\ndiff to v3:\n* Removed git-p4 OS X test fixes and submitted them in a separate roll as\n request by Junio [1].\n* Fix brew formula typo.\n (thanks Eric)\n* Move \"make configure && ./configure\" from \"install\" to \"before_script\" phase.\n (thanks Sebastian)\n* Run a build configuration with all kinds of \"NO_*\" flags. I don't understand\n all of them. Please let me know if some of them don't make sense. I also\n noticed that \"NO_ICONV\" will cause various tests to fail [2]. Is this a bug\n that should be fixed as part of this roll? If this is no bug, would it make\n sense to remove this flag?\n (thanks Junio, Matthieu)\n* Run make with multiple jobs. I found that -j2 is the fastest for building\n git on Travis. -j6 is the fastest to run the unit tests. I limited that to\n -j3 because I had the impression that I got more test failures with higher\n utilization.\n (thanks Jean-Noel, Matthieu)\n* Run Linux builds on new Travis CI container infrastructure .\n (thanks Matthieu)\n* Print Perforce/Git-LFS binary versions\n* Run tests with test extension flags enabled. Unfortunately \"GIT_TEST_LONG\"\n caused \"t0021\" to fail on Linux [3]. I wasn't able to reproduce this on my\n local machine, yet. Ideas anyone? Furthermore \"GETTEXT_POISON\" causes a bunch\n of failed tests on Linux and OS X [4]. Is this intentional or a bug?\n* Print verbose output of failed tests in the \"after_failure\" phase. Please\n note the little triangle on the left in the TravisCI log to expand the failed\n test output.\n* Update LFS to 1.0.2\n* Accept SIGPIPE output in some tests (similar to [5] by Junio)\n* Make git-p4 tests more robust with a general timeout and retry logic for\n cleanup commands (I now the \"rm\" and \"mkdir\" retry looks stupid but I was\n able to reproduce this error on TravisCI consistently - does anyone have\n a possible explanation why this could be necessary? Open file handles?)\n* Disable email notifications (at least initially). I found and fixed a few\n issues that caused flaky tests. However, I expect there are more and we can\n only find them over time. If you add this patch then TravisCI can run\n silently in the background. If people want to look at the TravisCI result\n then they can. All other people are not bothered by emails. If we find in\n six months from now that TravisCI has too many false failures then we could\n remove the .travis.yml, again, and move on.\n\nI had to apply this patch on master to get David Turner's fix for flaky\nt7063 [6]/[7].\n\nYou can find the TravisCI run for this roll here:\nhttps://travis-ci.org/larsxschneider/git/builds/89598194\n\nCheers,\nLars\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/279348/focus=279362\n[2] https://travis-ci.org/larsxschneider/git/builds/89555114\n[3] https://travis-ci.org/larsxschneider/git/builds/89554983\n[4] https://travis-ci.org/larsxschneider/git/builds/89554899\n[5] http://thread.gmane.org/gmane.comp.version-control.git/280120/focus=280543\n[6] http://thread.gmane.org/gmane.comp.version-control.git/279647\n[7] http://thread.gmane.org/gmane.comp.version-control.git/279889\n\nLars Schneider (4):\n add function test_must_fail_or_sigpipe and use it to fix flaky tests\n git-p4: add p4d timeout in tests\n git-p4: retry kill/cleanup operations in tests with timeout\n Add Travis CI support\n\n .travis.yml | 131 ++++++++++++++++++++++++++++++++++++++++\n t/lib-git-p4.sh | 51 +++++++++++++---\n t/t5504-fetch-receive-strict.sh | 3 +-\n t/t5516-fetch-push.sh | 8 +--\n t/test-lib-functions.sh | 23 +++++++\n 5 files changed, 201 insertions(+), 15 deletions(-)\n create mode 100644 .travis.yml\n\n--\n2.5.1\n"},{"id":"272998","messageId":"1446800323-2914-2-git-send-email-larsxschneider@gmail.com","threadId":"40738","inReplyTo":"1446800323-2914-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v4 1/4] add function test_must_fail_or_sigpipe and use it to fix flaky tests","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-06T08:58:40Z","receivedAt":"2015-11-06T08:58:40Z","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 SIGPIPE.\n\nThis patch accepts the SIGPIPE exit as legitimate test exit.\n\nSigned-off-by: Lars Schneider \n---\n t/t5504-fetch-receive-strict.sh | 3 +--\n t/t5516-fetch-push.sh | 8 ++++----\n t/test-lib-functions.sh | 23 +++++++++++++++++++++++\n 3 files changed, 28 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh\nindex 44f3d5f..129efa8 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_or_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..22a941b 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_or_sigpipe git fetch ../testrepo/.git $SHA1_3 &&\n+\t\t\ttest_must_fail_or_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\ttest_must_fail_or_sigpipe 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_or_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 73e37a1..19a598e 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -600,6 +600,29 @@ test_must_fail () {\n \treturn 0\n }\n\n+# Similar to test_must_fail, but tolerates sigpipe signal, too.\n+\n+test_must_fail_or_sigpipe () {\n+\t\"$@\"\n+\texit_code=$?\n+\tif test $exit_code = 0; then\n+\t\techo >&2 \"test_must_fail: command succeeded: $*\"\n+\t\treturn 1\n+\telif test $exit_code -ne 141 && \\\n+\t\t test $exit_code -gt 129 && \\\n+\t\t test $exit_code -le 192; then\n+\t\techo >&2 \"test_must_fail: died by signal: $*\"\n+\t\treturn 1\n+\telif test $exit_code = 127; then\n+\t\techo >&2 \"test_must_fail: command not found: $*\"\n+\t\treturn 1\n+\telif test $exit_code = 126; then\n+\t\techo >&2 \"test_must_fail: valgrind error: $*\"\n+\t\treturn 1\n+\tfi\n+\treturn 0\n+}\n+\n # Similar to test_must_fail, but tolerates success, too. This is\n # meant to be used in contexts like:\n #\n--\n2.5.1\n"},{"id":"273000","messageId":"1446800323-2914-3-git-send-email-larsxschneider@gmail.com","threadId":"40738","inReplyTo":"1446800323-2914-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v4 2/4] git-p4: add p4d timeout in tests","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-06T08:58:41Z","receivedAt":"2015-11-06T08:58:41Z","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 7548225..b1660f6 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+# 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@@ -81,6 +85,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+ while 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@@ -131,7 +148,8 @@ kill_p4d() {\n \tdone &&\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":"272999","messageId":"1446800323-2914-4-git-send-email-larsxschneider@gmail.com","threadId":"40738","inReplyTo":"1446800323-2914-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v4 3/4] git-p4: retry kill/cleanup operations in tests with timeout","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-06T08:58:42Z","receivedAt":"2015-11-06T08:58:42Z","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 b1660f6..9043b51 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 # the defined timeout in seconds.\n P4D_TIMEOUT=300\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@@ -138,14 +142,24 @@ 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@@ -153,8 +167,9 @@ kill_p4d() {\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":"272997","messageId":"1446800323-2914-5-git-send-email-larsxschneider@gmail.com","threadId":"40738","inReplyTo":"1446800323-2914-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v4 4/4] Add Travis CI support","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-06T08:58:43Z","receivedAt":"2015-11-06T08:58:43Z","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":"273001","messageId":"CAPig+cSnhxEQdjiLt8+hjMnYYAxaLRVAYX-S5s3BHTpFOMcWqA@mail.gmail.com","threadId":"40738","inReplyTo":"1446800323-2914-3-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v4 2/4] git-p4: add p4d timeout in tests","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-11-06T09:23:15Z","receivedAt":"2015-11-06T09:23:15Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Nov 6, 2015 at 3:58 AM, wrote:\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>\n> Signed-off-by: Lars Schneider \n> ---\n> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n> @@ -81,6 +85,19 @@ start_p4d() {\n> # will be caught with the \"kill -0\" check below.\n> i=${P4D_START_PATIENCE:-300}\n> pid=$(cat \"$pidfile\")\n> +\n> + timeout=$(($(date +%s) + $P4D_TIMEOUT))\n> + while true\n\nThe 'while' line is incorrectly indented with spaces rather than tabs.\n\n> + do\n> + if test $(date +%s) -gt $timeout\n\nI don't know how portable you intend this to be, but 'date +%s' is not\nuniversally supported (it's missing on Solaris, for instance, and\nperhaps AIX). For 6a9d16a (filter-branch: add passed/remaining seconds\non progress, 2015-09-07), we ultimately settled upon detecting +%s\nsupport dynamically:\n\n if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'\n # it's supported\n fi\n\nPerhaps you'd want to detect this via a lazy prerequisite and skip\nthis if not supported?\n\n> + then\n> + kill -9 $pid\n> + exit 1\n> + fi\n> + sleep 1\n> + done &\n> + watchdog_pid=$!\n> +\n> ready=\n> while test $i -gt 0\n> do\n> @@ -131,7 +148,8 @@ kill_p4d() {\n> done &&\n> # complain if it would not die\n> test_must_fail kill $pid >/dev/null 2>&1 &&\n> - rm -rf \"$db\" \"$cli\" \"$pidfile\"\n> + rm -rf \"$db\" \"$cli\" \"$pidfile\" &&\n> + retry_until_fail kill -9 $watchdog_pid\n> }\n>\n> cleanup_git() {\n> --\n> 2.5.1\n"},{"id":"273002","messageId":"CAPig+cQiGqo2rNSC_Lw-WXq0ABp6PRFV-2w1wtK+XccQt2RD=w@mail.gmail.com","threadId":"40738","inReplyTo":"1446800323-2914-4-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v4 3/4] git-p4: retry kill/cleanup operations in tests with timeout","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-11-06T09:28:03Z","receivedAt":"2015-11-06T09:28:03Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Nov 6, 2015 at 3:58 AM, wrote:\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> +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\nI'm confused by this. Patch 2/4 was already calling\nretry_until_fail(), but it's introduction seems to be here in patch\n3/4. Am I missing something obvious?\n\n> kill_p4d() {\n> pid=$(cat \"$pidfile\")\n> - # it had better exist for the first kill\n> - kill $pid &&\n> - for i in 1 2 3 4 5 ; do\n> - kill $pid >/dev/null 2>&1 || break\n> - sleep 1\n> - done &&\n> + retry_until_fail kill $pid\n> + retry_until_fail kill -9 $pid\n> # complain if it would not die\n> test_must_fail kill $pid >/dev/null 2>&1 &&\n> rm -rf \"$db\" \"$cli\" \"$pidfile\" &&\n> @@ -153,8 +167,9 @@ kill_p4d() {\n> }\n>\n> cleanup_git() {\n> - rm -rf \"$git\" &&\n> - mkdir \"$git\"\n> + retry_until_success rm -r \"$git\"\n> + test_must_fail test -d \"$git\" &&\n> + retry_until_success mkdir \"$git\"\n> }\n>\n> marshal_dump() {\n> --\n> 2.5.1\n"},{"id":"273003","messageId":"624E4E74-6EEF-471E-9F12-EFE69FB2A7AA@gmail.com","threadId":"40738","inReplyTo":"CAPig+cQiGqo2rNSC_Lw-WXq0ABp6PRFV-2w1wtK+XccQt2RD=w@mail.gmail.com","subject":"Re: [PATCH v4 3/4] git-p4: retry kill/cleanup operations in tests with timeout","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-06T09:47:43Z","receivedAt":"2015-11-06T09:47:43Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 06 Nov 2015, at 10:28, Eric Sunshine wrote:\n> \n> On Fri, Nov 6, 2015 at 3:58 AM, wrote:\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>> +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> I'm confused by this. Patch 2/4 was already calling\n> retry_until_fail(), but it's introduction seems to be here in patch\n> 3/4. Am I missing something obvious?\n\nNo, my fault. I reordered the commits and forgot about this. I will fix the order in the next roll.\n\nThanks,\nLars"},{"id":"273005","messageId":"CAPig+cSc+BBgd3-UeVE79D9GWXfaQqbLCQdKo6FbhnPMsK_eFA@mail.gmail.com","threadId":"40738","inReplyTo":"1446800323-2914-5-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v4 4/4] Add Travis CI support","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-11-06T09:56:17Z","receivedAt":"2015-11-06T09:56:17Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Nov 6, 2015 at 3:58 AM, wrote:\n> The tests are currently executed on \"Ubuntu 12.04 LTS Server Edition\n> 64 bit\" and on \"OS X Mavericks\" using gcc and clang.\n>\n> Perforce and Git-LFS are installed and therefore available for the\n> respective tests.\n>\n> Signed-off-by: Lars Schneider \n> ---\n> diff --git a/.travis.yml b/.travis.yml\n> @@ -0,0 +1,131 @@\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\nHmm, aren't you losing test coverage by disabling some of these? Is\nthat desirable for continuous integration testing? Am I missing\nsomething?\n"},{"id":"273008","messageId":"AF36AA68-F603-4151-8314-C9BA69EB27D0@gmail.com","threadId":"40738","inReplyTo":"CAPig+cSc+BBgd3-UeVE79D9GWXfaQqbLCQdKo6FbhnPMsK_eFA@mail.gmail.com","subject":"Re: [PATCH v4 4/4] Add Travis CI support","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-06T13:18:59Z","receivedAt":"2015-11-06T13:18:59Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 06 Nov 2015, at 10:56, Eric Sunshine wrote:\n> \n> On Fri, Nov 6, 2015 at 3:58 AM, wrote:\n>> The tests are currently executed on \"Ubuntu 12.04 LTS Server Edition\n>> 64 bit\" and on \"OS X Mavericks\" using gcc and clang.\n>> \n>> Perforce and Git-LFS are installed and therefore available for the\n>> respective tests.\n>> \n>> Signed-off-by: Lars Schneider \n>> ---\n>> diff --git a/.travis.yml b/.travis.yml\n>> @@ -0,0 +1,131 @@\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> Hmm, aren't you losing test coverage by disabling some of these? Is\n> that desirable for continuous integration testing? Am I missing\n> something?\nPer platform/compiler (Linux&Mac/clang&gcc) we run two configurations. One normal configuration (see the lonely \"-\" right under \"matrix:\") and one configuration with all sorts of things are disabled (\"NO...\").\n\nYou can see all 8 configurations ([linux, mac] * [clang, gcc] * [normal, NO_...]) here:\nhttps://travis-ci.org/larsxschneider/git/builds/89598194\n\nCheers,\nLars \n"},{"id":"273009","messageId":"493416AE-4514-437F-909D-BE2894FF118D@gmail.com","threadId":"40738","inReplyTo":"CAPig+cQiGqo2rNSC_Lw-WXq0ABp6PRFV-2w1wtK+XccQt2RD=w@mail.gmail.com","subject":"Re: [PATCH v4 3/4] git-p4: retry kill/cleanup operations in tests with timeout","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-06T13:19:37Z","receivedAt":"2015-11-06T13:19:37Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 06 Nov 2015, at 10:28, Eric Sunshine wrote:\n> \n> On Fri, Nov 6, 2015 at 3:58 AM, wrote:\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>> +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> I'm confused by this. Patch 2/4 was already calling\n> retry_until_fail(), but it's introduction seems to be here in patch\n> 3/4. Am I missing something obvious?\nNo, my fault. I reordered the commits and forgot about this. I will fix the order in the next roll.\n\nThanks,\nLars\n"},{"id":"273010","messageId":"CAHGBnuNwzGNjfh57zFOtwr6qU1PCsc7rgOniY8-Mz5Jgnv2QTg@mail.gmail.com","threadId":"40738","inReplyTo":"22B2C2B1-9260-4EC0-A4C5-C7F7DDD388BA@gmail.com","subject":"Re: [PATCH v4 4/4] Add Travis CI support","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2015-11-06T13:20:09Z","receivedAt":"2015-11-06T13:20:09Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Fri, Nov 6, 2015 at 2:11 PM, Lars Schneider wrote:\n\n> Per platform/compiler (Linux&Mac/clang&gcc) we run two configurations. One\n> normal configuration (see the lonely \"-\" right under \"matrix:\") and one\n> configuration with all sorts of things are disabled (\"NO...\").\n>\n> You can see all 8 configurations ([linux, mac] * [clang, gcc] * [normal,\n> NO_...]) here:\n> https://travis-ci.org/larsxschneider/git/builds/89598194\n\nAren't these 8 configurations a bit too much? I see the total running\ntime is about 2 hours. For my taste, this is way to much to give\ndevelopers feedback about the status of their PR. It should be\nsomething < 30 minutes.\n\nIMO, the purpose of the Travis CI configuration mainly is to 1) save\ndevelopers work by not requiring to build manually, 2) build on other\nplatforms than the developer has access to. I doubt that the average\ndeveloper manually builds anything close to these 8 configurations\nbefore we had this job, so I wouldn't make it a requirement for Travis\nto do much more than a developer could / would to manually.\n\nOn the other hand, I see the point in letting a CI system test more\nconfigurations than manually feasible. But that should be done as part\nof a different job then. E.g. we could have a \"fast\" PR validation\njob, and a \"slow\" nightly build job.\n\n-- \nSebastian Schuberth\n"},{"id":"273011","messageId":"499A69EE-4037-4865-8707-44157AD5A32C@gmail.com","threadId":"40738","inReplyTo":"CAPig+cSnhxEQdjiLt8+hjMnYYAxaLRVAYX-S5s3BHTpFOMcWqA@mail.gmail.com","subject":"Re: [PATCH v4 2/4] git-p4: add p4d timeout in tests","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-06T13:20:38Z","receivedAt":"2015-11-06T13:20:38Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 06 Nov 2015, at 10:23, Eric Sunshine wrote:\n> \n> On Fri, Nov 6, 2015 at 3:58 AM, wrote:\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>> \n>> Signed-off-by: Lars Schneider \n>> ---\n>> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n>> @@ -81,6 +85,19 @@ start_p4d() {\n>> # will be caught with the \"kill -0\" check below.\n>> i=${P4D_START_PATIENCE:-300}\n>> pid=$(cat \"$pidfile\")\n>> +\n>> + timeout=$(($(date +%s) + $P4D_TIMEOUT))\n>> + while true\n> \n> The 'while' line is incorrectly indented with spaces rather than tabs.'\n\nOh, you're right. One more thing that I will add to my pre mailing list submit check script :-)\n\n> \n>> + do\n>> + if test $(date +%s) -gt $timeout\n> \n> I don't know how portable you intend this to be, but 'date +%s' is not\n> universally supported (it's missing on Solaris, for instance, and\n> perhaps AIX). For 6a9d16a (filter-branch: add passed/remaining seconds\n> on progress, 2015-09-07), we ultimately settled upon detecting +%s\n> support dynamically:\n> \n> if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'\n> # it's supported\n> fi\n> \n> Perhaps you'd want to detect this via a lazy prerequisite and skip\n> this if not supported?\n\nAFAIK Perforce does not run on Solaris and AIX anyways (see supported platforms [1]). Therefore these tests should not be executed on these platforms. A lazy prerequisite sounds like a good idea to make this explicit!\n\nThanks,\nLars\n\n[1] https://www.perforce.com/perforce/doc.current/user/relnotes.txt\n"},{"id":"273012","messageId":"D71ED634-382A-4835-AF65-D16FEFF17270@gmail.com","threadId":"40738","inReplyTo":"CAHGBnuNwzGNjfh57zFOtwr6qU1PCsc7rgOniY8-Mz5Jgnv2QTg@mail.gmail.com","subject":"Re: [PATCH v4 4/4] Add Travis CI support","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-06T13:28:37Z","receivedAt":"2015-11-06T13:28:37Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 06 Nov 2015, at 14:20, Sebastian Schuberth wrote:\n> \n> On Fri, Nov 6, 2015 at 2:11 PM, Lars Schneider wrote:\n> \n>> Per platform/compiler (Linux&Mac/clang&gcc) we run two configurations. One\n>> normal configuration (see the lonely \"-\" right under \"matrix:\") and one\n>> configuration with all sorts of things are disabled (\"NO...\").\n>> \n>> You can see all 8 configurations ([linux, mac] * [clang, gcc] * [normal,\n>> NO_...]) here:\n>> https://travis-ci.org/larsxschneider/git/builds/89598194\n> \n> Aren't these 8 configurations a bit too much? I see the total running\n> time is about 2 hours. For my taste, this is way to much to give\n> developers feedback about the status of their PR. It should be\n> something < 30 minutes.\n> \n> IMO, the purpose of the Travis CI configuration mainly is to 1) save\n> developers work by not requiring to build manually, 2) build on other\n> platforms than the developer has access to. I doubt that the average\n> developer manually builds anything close to these 8 configurations\n> before we had this job, so I wouldn't make it a requirement for Travis\n> to do much more than a developer could / would to manually.\n> \n> On the other hand, I see the point in letting a CI system test more\n> configurations than manually feasible. But that should be done as part\n> of a different job then. E.g. we could have a \"fast\" PR validation\n> job, and a \"slow\" nightly build job.\n> \n\nWell, I partly agree. Right now the running time is ~20 min (that means less than your 30min target!). After ~10min you even have all Linux results, Mac takes a bit longer. Travis shows you 2h because that is the time that would be required if all builds where run one after another (we run builds in parallel).\n\nThat being said, I see your point of to avoiding to burn Travis CI resources meaningless. If I am not mistaken then you can configure Travis in a way that it runs different configurations for different branches. E.g. I would like to run all 8 configurations on maint, master, next and maybe pu. All other branches on peoples own forks should be fine with the default Linux build (~10min).\n\nWhat do you think?\n\nThanks,\nLars"},{"id":"273014","messageId":"CAHGBnuO9aVGYLd+3F3wp7QwazQvWZAS-NeNba3SwsuVkcVkCWg@mail.gmail.com","threadId":"40738","inReplyTo":"D71ED634-382A-4835-AF65-D16FEFF17270@gmail.com","subject":"Re: [PATCH v4 4/4] Add Travis CI support","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2015-11-06T13:36:59Z","receivedAt":"2015-11-06T13:36:59Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Fri, Nov 6, 2015 at 2:28 PM, Lars Schneider wrote:\n\n> Well, I partly agree. Right now the running time is ~20 min (that means less than your 30min target!). After ~10min you even have all Linux results, Mac takes a bit longer. Travis shows you 2h because that is the time that would be required if all builds where run one after another (we run builds in parallel).\n\nAre you sure about than? I mean, what sense does it make to show how\nlong it *would* have taken *if* the builds were running serially? I\ncan see that the longest of the jobs took 21 minutes, which is ok. But\nthat does not mean that all jobs completed in within 21 minutes. It\ncould be that not all jobs started at (about) the same time due to a\nlack of resources, and that the last job did not compete before the 2\nhours were over because it only started to run 1 hours and 40 minutes\nbefor ethe first job was started.\n\n> That being said, I see your point of to avoiding to burn Travis CI resources meaningless. If I am not mistaken then you can configure Travis in a way that it runs different configurations for different branches. E.g. I would like to run all 8 configurations on maint, master, next and maybe pu. All other branches on peoples own forks should be fine with the default Linux build (~10min).\n>\n> What do you think?\n\nI think running different configuration per branch makes sense, yes.\n\n-- \nSebastian Schuberth\n"},{"id":"273016","messageId":"BE90093A-90AA-4E63-9664-C5ABCDE6B410@gmail.com","threadId":"40738","inReplyTo":"CAHGBnuO9aVGYLd+3F3wp7QwazQvWZAS-NeNba3SwsuVkcVkCWg@mail.gmail.com","subject":"Re: [PATCH v4 4/4] Add Travis CI support","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-06T13:55:44Z","receivedAt":"2015-11-06T13:55:44Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 06 Nov 2015, at 14:36, Sebastian Schuberth wrote:\n> \n> On Fri, Nov 6, 2015 at 2:28 PM, Lars Schneider wrote:\n> \n>> Well, I partly agree. Right now the running time is ~20 min (that means less than your 30min target!). After ~10min you even have all Linux results, Mac takes a bit longer. Travis shows you 2h because that is the time that would be required if all builds where run one after another (we run builds in parallel).\n> \n> Are you sure about than? I mean, what sense does it make to show how\n> long it *would* have taken *if* the builds were running serially? I\n> can see that the longest of the jobs took 21 minutes, which is ok. But\n> that does not mean that all jobs completed in within 21 minutes. It\n> could be that not all jobs started at (about) the same time due to a\n> lack of resources, and that the last job did not compete before the 2\n> hours were over because it only started to run 1 hours and 40 minutes\n> befor ethe first job was started.\nI am fairly certain about this. \n\nSee, here is the first configuration and the first test case of a job:\nhttps://travis-ci.org/larsxschneider/git/jobs/89598195\n[08:21:06] t0002-gitfile.sh \n\nHere is the last configuration and the last test case of the same job:\n[08:51:22] t9903-bash-prompt.sh\n\n~30 min for all 8 configurations. You can enable Travis CI for you GitHub account and try it easily yourself :-)\n\n> \n>> That being said, I see your point of to avoiding to burn Travis CI resources meaningless. If I am not mistaken then you can configure Travis in a way that it runs different configurations for different branches. E.g. I would like to run all 8 configurations on maint, master, next and maybe pu. All other branches on peoples own forks should be fine with the default Linux build (~10min).\n>> \n>> What do you think?\n> \n> I think running different configuration per branch makes sense, yes.\nIf the list decides to accept this patch. Do you think that would be a necessary requirement for the first iteration?\n\nThanks,\nLars"},{"id":"273017","messageId":"CAHGBnuOmZFYUXKRnydUZSouOuJ59QU8K_=FvQG0xkQ+TywFPew@mail.gmail.com","threadId":"40738","inReplyTo":"BE90093A-90AA-4E63-9664-C5ABCDE6B410@gmail.com","subject":"Re: [PATCH v4 4/4] Add Travis CI support","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2015-11-06T13:57:54Z","receivedAt":"2015-11-06T13:57:54Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Fri, Nov 6, 2015 at 2:55 PM, Lars Schneider wrote:\n\n>> I think running different configuration per branch makes sense, yes.\n>\n> If the list decides to accept this patch. Do you think that would be a necessary requirement for the first iteration?\n\nNo. I think this could be addressed later as an improvements. To me\nit's more important to finally get *some* sane Travis CI configuration\nin.\n\n-- \nSebastian Schuberth\n"},{"id":"273018","messageId":"52B7821B-A46D-4DEC-9102-B5573DE53113@gmail.com","threadId":"40738","inReplyTo":"CAHGBnuOmZFYUXKRnydUZSouOuJ59QU8K_=FvQG0xkQ+TywFPew@mail.gmail.com","subject":"Re: [PATCH v4 4/4] Add Travis CI support","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-11-06T14:08:36Z","receivedAt":"2015-11-06T14:08:36Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 06 Nov 2015, at 14:57, Sebastian Schuberth wrote:\n> \n> On Fri, Nov 6, 2015 at 2:55 PM, Lars Schneider wrote:\n> \n>>> I think running different configuration per branch makes sense, yes.\n>> \n>> If the list decides to accept this patch. Do you think that would be a necessary requirement for the first iteration?\n> \n> No. I think this could be addressed later as an improvements. To me\n> it's more important to finally get *some* sane Travis CI configuration\n> in.\nTrue. However, as I stated in my v4 cover letter the Travis CI integration is not yet perfect. I am constantly running builds to find flaky tests. Eg. here is one of them in git-p4 area that I will tackle next:\nhttps://s3.amazonaws.com/archive.travis-ci.org/jobs/89603763/log.txt\n\nI also see a weird \"prove Tests our of sequence\" error one in a while:\nhttps://s3.amazonaws.com/archive.travis-ci.org/jobs/89603770/log.txt\n\nDoes anyone have an idea what could cause this?\n\nThanks,\nLars\n"},{"id":"273026","messageId":"xmqqr3k32dc9.fsf@gitster.mtv.corp.google.com","threadId":"40738","inReplyTo":"1446800323-2914-2-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v4 1/4] add function test_must_fail_or_sigpipe and use it to fix flaky tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-11-06T18:27:02Z","receivedAt":"2015-11-06T18:27:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"larsxschneider@gmail.com writes:\n\n> From: Lars Schneider \n>\n> t5516 \"75 - deny fetch unreachable SHA1, allowtipsha1inwant=true\" is\n> flaky in the following case:\n> 1. remote upload-pack finds out \"not our ref\"\n> 2. remote sends a response and closes the pipe\n> 3. fetch-pack still tries to write commands to the remote upload-pack\n> 4. write call in wrapper.c dies with SIGPIPE\n>\n> t5504 \"9 - push with transfer.fsckobjects\" is flaky, too, and returns\n> SIGPIPE once in a while. I had to remove the final \"To dst...\" output\n> check because there is no output if the process dies with SIGPIPE.\n\nThanks for a clear write-up.\n\n> This patch accepts the SIGPIPE exit as legitimate test exit.\n\nIt is not this patch that accepts such a failure as legit ;-).\n\n\"Accept such a death-with-sigpipe also as OK when we are expecting a\nfailure\", perhaps?\n\n\n\n> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\n> index ec22c98..22a941b 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_or_sigpipe git fetch ../testrepo/.git $SHA1_3 &&\n> +\t\t\ttest_must_fail_or_sigpipe git fetch ../testrepo/.git $SHA1_1 &&\n> \t\t\tgit --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n\nThese I understand.\n\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\ttest_must_fail_or_sigpipe git cat-file commit $SHA1_2 &&\n\nThis I don't. Under what condition is it sane for this \"cat-file\ncommit\" to fail with sigpipe?\n\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_or_sigpipe git fetch ../testrepo/.git $SHA1_3\n\nAnd this I do.\n\n> \t\t)\n> \t'\n> done\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> index 73e37a1..19a598e 100644\n> --- a/t/test-lib-functions.sh\n> +++ b/t/test-lib-functions.sh\n> @@ -600,6 +600,29 @@ test_must_fail () {\n> \treturn 0\n> }\n>\n> +# Similar to test_must_fail, but tolerates sigpipe signal, too.\n> +\n> +test_must_fail_or_sigpipe () {\n> +\t\"$@\"\n> +\texit_code=$?\n> +\tif test $exit_code = 0; then\n> +\t\techo >&2 \"test_must_fail: command succeeded: $*\"\n> +\t\treturn 1\n> +\telif test $exit_code -ne 141 && \\\n> +\t\t test $exit_code -gt 129 && \\\n> +\t\t test $exit_code -le 192; then\n> +\t\techo >&2 \"test_must_fail: died by signal: $*\"\n> +\t\treturn 1\n> +\telif test $exit_code = 127; then\n> +\t\techo >&2 \"test_must_fail: command not found: $*\"\n> +\t\treturn 1\n> +\telif test $exit_code = 126; then\n> +\t\techo >&2 \"test_must_fail: valgrind error: $*\"\n> +\t\treturn 1\n> +\tfi\n> +\treturn 0\n> +}\n> +\n\nWhen you are coming up with this patch, you must have checked the\nexisting code around this area. Did you notice that 126 is handled\ndifferently between must_fail and might_fail? Can you explain and\njustify the difference?\n\nOne explanation I can think of is that the contributor who did\neeb69131 (tests: notice valgrind error in test_must_fail,\n2013-03-31) did not notice that we do essentially the same thing in\nmight_fail and forgot to add it. And it is not the fault of that\ncontributor, but the fault of the duplicated and poorly organized\ncode back then.\n\nAdding the third variant in the way this patch does is making things\nworse by inviting more mistakes.\n\nHow about doing something like the attached to consolidate the\nexisting two into one, and then build this third one on top?\n\n t/test-lib-functions.sh | 41 +++++++++++++++++++++--------------------\n 1 file changed, 21 insertions(+), 20 deletions(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex e8d3c0f..54497d6 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -560,22 +560,7 @@ test_line_count () {\n # the failure could be due to a segv. We want a controlled failure.\n \n test_must_fail () {\n-\t\"$@\"\n-\texit_code=$?\n-\tif test $exit_code = 0; then\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\techo >&2 \"test_must_fail: died by signal: $*\"\n-\t\treturn 1\n-\telif test $exit_code = 127; then\n-\t\techo >&2 \"test_must_fail: command not found: $*\"\n-\t\treturn 1\n-\telif test $exit_code = 126; then\n-\t\techo >&2 \"test_must_fail: valgrind error: $*\"\n-\t\treturn 1\n-\tfi\n-\treturn 0\n+\ttest_failure_helper test_must_fail \"$@\"\n }\n \n # Similar to test_must_fail, but tolerates success, too. This is\n@@ -590,13 +575,29 @@ test_must_fail () {\n # because we want to notice if it fails due to segv.\n \n test_might_fail () {\n+\ttest_failure_helper test_might_fail \"$@\"\n+}\n+\n+test_failure_helper () {\n+\ttest_expect=$1\n+\tshift\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+\tif test $test_expect != test_might_fail && test $exit_code = 0\n+\tthen\n+\t\techo >&2 \"$test_expect: command succeeded: $*\"\n+\t\treturn 1\n+\telif test $exit_code -gt 129 && test $exit_code -le 192\n+\tthen\n+\t\techo >&2 \"$test_expect: died by signal: $*\"\n+\t\treturn 1\n+\telif test $exit_code = 127\n+\tthen\n+\t\techo >&2 \"$test_expect: command not found: $*\"\n \t\treturn 1\n-\telif test $exit_code = 127; then\n-\t\techo >&2 \"test_might_fail: command not found: $*\"\n+\telif test $exit_code = 126\n+\tthen\n+\t\techo >&2 \"$test_expect: valgrind error: $*\"\n \t\treturn 1\n \tfi\n \treturn 0\n"},{"id":"273028","messageId":"xmqqmvur2cb8.fsf@gitster.mtv.corp.google.com","threadId":"40738","inReplyTo":"xmqqr3k32dc9.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 1/4] add function test_must_fail_or_sigpipe and use it to fix flaky tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-11-06T18:49:15Z","receivedAt":"2015-11-06T18:49:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Adding the third variant in the way this patch does is making things\n> worse by inviting more mistakes.\n>\n> How about doing something like the attached to consolidate the\n> existing two into one, and then build this third one on top?\n\nActually, I think this other variant I came up with is cleaner and\nis more easily extensible. It already has the support for sigpipe,\ne.g. you can do\n\n\ttest_must_fail ok=sigpipe git fetch ...\n\nOf course, you can do\n\n\ttest_must_fail_or_sigpipe () {\n\t\ttest_must_fail ok=sigpipe \"$@\"\n\t}\n\nif it is easier to read. I do not have a very strong opinion either\nway.\n\n t/test-lib-functions.sh | 28 +++++++++++++++++-----------\n 1 file changed, 17 insertions(+), 11 deletions(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex e8d3c0f..b732f87 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -560,11 +560,26 @@ 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 test $exit_code = 0\n+\tthen\n \t\techo >&2 \"test_must_fail: command succeeded: $*\"\n \t\treturn 1\n+\telif ! case \",$_test_ok,\" in *,sigpipe,*) false;; esac &&\n+\t test $exit_code = 141\n+\tthen\n+\t\treturn 0\n \telif test $exit_code -gt 129 && test $exit_code -le 192; then\n \t\techo >&2 \"test_must_fail: died by signal: $*\"\n \t\treturn 1\n@@ -590,16 +605,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"}]}