threads / patch / 40738

v4, 4 partsAdd Travis CI support

Subject: [PATCH v4 0/4] Add Travis CI support

## tl;dr

20 messages between Nov 6, 2015 and Nov 6, 2015. Diffs are folded; open one to read it.

replies: 19people: 4as markdown or json

larsxschneider@gmail.com· Nov 6, 2015, 08:58 UTC · lore
From: Lars Schneider <larsxschneider@gmail.com>
diff to v3:
* Removed git-p4 OS X test fixes and submitted them in a separate roll as
  request by Junio [1].
* Fix brew formula typo.
  (thanks Eric)
* Move "make configure && ./configure" from "install" to "before_script" phase.
  (thanks Sebastian)
* Run a build configuration with all kinds of "NO_*" flags. I don't understand
  all of them. Please let me know if some of them don't make sense. I also
  noticed that "NO_ICONV" will cause various tests to fail [2]. Is this a bug
  that should be fixed as part of this roll? If this is no bug, would it make
  sense to remove this flag?
  (thanks Junio, Matthieu)
* Run make with multiple jobs. I found that -j2 is the fastest for building
  git on Travis. -j6 is the fastest to run the unit tests. I limited that to
  -j3 because I had the impression that I got more test failures with higher
  utilization.
  (thanks Jean-Noel, Matthieu)
* Run Linux builds on new Travis CI container infrastructure .
  (thanks Matthieu)
* Print Perforce/Git-LFS binary versions
* Run tests with test extension flags enabled. Unfortunately "GIT_TEST_LONG"
  caused "t0021" to fail on Linux [3]. I wasn't able to reproduce this on my
  local machine, yet. Ideas anyone? Furthermore "GETTEXT_POISON" causes a bunch
  of failed tests on Linux and OS X [4]. Is this intentional or a bug?
* Print verbose output of failed tests in the "after_failure" phase. Please
  note the little triangle on the left in the TravisCI log to expand the failed
  test output.
* Update LFS to 1.0.2
* Accept SIGPIPE output in some tests (similar to [5] by Junio)
* Make git-p4 tests more robust with a general timeout and retry logic for
  cleanup commands (I now the "rm" and "mkdir" retry looks stupid but I was
  able to reproduce this error on TravisCI consistently - does anyone have
  a possible explanation why this could be necessary? Open file handles?)
* Disable email notifications (at least initially). I found and fixed a few
  issues that caused flaky tests. However, I expect there are more and we can
  only find them over time. If you add this patch then TravisCI can run
  silently in the background. If people want to look at the TravisCI result
  then they can. All other people are not bothered by emails. If we find in
  six months from now that TravisCI has too many false failures then we could
  remove the .travis.yml, again, and move on.

I had to apply this patch on master to get David Turner's fix for flaky t7063 [6]/[7].

You can find the TravisCI run for this roll here: https://travis-ci.org/larsxschneider/git/builds/89598194

Cheers, Lars

[1] http://thread.gmane.org/gmane.comp.version-control.git/279348/focus=279362 [2] https://travis-ci.org/larsxschneider/git/builds/89555114 [3] https://travis-ci.org/larsxschneider/git/builds/89554983 [4] https://travis-ci.org/larsxschneider/git/builds/89554899 [5] http://thread.gmane.org/gmane.comp.version-control.git/280120/focus=280543 [6] http://thread.gmane.org/gmane.comp.version-control.git/279647 [7] http://thread.gmane.org/gmane.comp.version-control.git/279889

Lars Schneider (4):
  add function test_must_fail_or_sigpipe and use it to fix flaky tests
  git-p4: add p4d timeout in tests
  git-p4: retry kill/cleanup operations in tests with timeout
  Add Travis CI support
 .travis.yml                     | 131 ++++++++++++++++++++++++++++++++++++++++
 t/lib-git-p4.sh                 |  51 +++++++++++++---
 t/t5504-fetch-receive-strict.sh |   3 +-
 t/t5516-fetch-push.sh           |   8 +--
 t/test-lib-functions.sh         |  23 +++++++
 5 files changed, 201 insertions(+), 15 deletions(-)
 create mode 100644 .travis.yml

-- 2.5.1

larsxschneider@gmail.com· Nov 6, 2015, 08:58 UTC · re: larsxschneider@gmail.com · lore

[PATCH v4 1/4] add function test_must_fail_or_sigpipe and use it to fix flaky tests

From: Lars Schneider <larsxschneider@gmail.com>
t5516 "75 - deny fetch unreachable SHA1, allowtipsha1inwant=true" is
flaky in the following case:
1. remote upload-pack finds out "not our ref"
2. remote sends a response and closes the pipe
3. fetch-pack still tries to write commands to the remote upload-pack
4. write call in wrapper.c dies with SIGPIPE

t5504 "9 - push with transfer.fsckobjects" is flaky, too, and returns SIGPIPE once in a while. I had to remove the final "To dst..." output check because there is no output if the process dies with SIGPIPE.

This patch accepts the SIGPIPE exit as legitimate test exit.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
 t/t5504-fetch-receive-strict.sh |  3 +--
 t/t5516-fetch-push.sh           |  8 ++++----
 t/test-lib-functions.sh         | 23 +++++++++++++++++++++++
 3 files changed, 28 insertions(+), 6 deletions(-)
Show changes to 3 files +28 −7

t/t5504-fetch-receive-strict.sh, t/t5516-fetch-push.sh, t/test-lib-functions.sh

diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
index 44f3d5f..129efa8 100755
--- a/t/t5504-fetch-receive-strict.sh
+++ b/t/t5504-fetch-receive-strict.sh
@@ -111,8 +111,7 @@ test_expect_success 'push with transfer.fsckobjects' '
 		cd dst &&
 		git config transfer.fsckobjects true
 	) &&
-	test_must_fail git push --porcelain dst master:refs/heads/test >act &&
-	test_cmp exp act
+	test_must_fail_or_sigpipe git push --porcelain dst master:refs/heads/test >act
 '

 cat >bogus-commit <<\EOF
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index ec22c98..22a941b 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1162,15 +1162,15 @@ do
 		mk_empty shallow &&
 		(
 			cd shallow &&
-			test_must_fail git fetch ../testrepo/.git $SHA1_3 &&
-			test_must_fail git fetch ../testrepo/.git $SHA1_1 &&
+			test_must_fail_or_sigpipe git fetch ../testrepo/.git $SHA1_3 &&
+			test_must_fail_or_sigpipe git fetch ../testrepo/.git $SHA1_1 &&
 			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
 			git fetch ../testrepo/.git $SHA1_1 &&
 			git cat-file commit $SHA1_1 &&
-			test_must_fail git cat-file commit $SHA1_2 &&
+			test_must_fail_or_sigpipe git cat-file commit $SHA1_2 &&
 			git fetch ../testrepo/.git $SHA1_2 &&
 			git cat-file commit $SHA1_2 &&
-			test_must_fail git fetch ../testrepo/.git $SHA1_3
+			test_must_fail_or_sigpipe git fetch ../testrepo/.git $SHA1_3
 		)
 	'
 done
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 73e37a1..19a598e 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -600,6 +600,29 @@ test_must_fail () {
 	return 0
 }

+# Similar to test_must_fail, but tolerates sigpipe signal, too.
+
+test_must_fail_or_sigpipe () {
+	"$@"
+	exit_code=$?
+	if test $exit_code = 0; then
+		echo >&2 "test_must_fail: command succeeded: $*"
+		return 1
+	elif test $exit_code -ne 141 && \
+		 test $exit_code -gt 129 && \
+		 test $exit_code -le 192; then
+		echo >&2 "test_must_fail: died by signal: $*"
+		return 1
+	elif test $exit_code = 127; then
+		echo >&2 "test_must_fail: command not found: $*"
+		return 1
+	elif test $exit_code = 126; then
+		echo >&2 "test_must_fail: valgrind error: $*"
+		return 1
+	fi
+	return 0
+}
+
 # Similar to test_must_fail, but tolerates success, too.  This is
 # meant to be used in contexts like:
 #
--
2.5.1
Junio C Hamano· Nov 6, 2015, 18:27 UTC · re: larsxschneider@gmail.com · lore

Re: [PATCH v4 1/4] add function test_must_fail_or_sigpipe and use it to fix flaky tests

larsxschneider@gmail.com writes:
Show 12 quoted lines
> From: Lars Schneider <larsxschneider@gmail.com>
>
> t5516 "75 - deny fetch unreachable SHA1, allowtipsha1inwant=true" is
> flaky in the following case:
> 1. remote upload-pack finds out "not our ref"
> 2. remote sends a response and closes the pipe
> 3. fetch-pack still tries to write commands to the remote upload-pack
> 4. write call in wrapper.c dies with SIGPIPE
>
> t5504 "9 - push with transfer.fsckobjects" is flaky, too, and returns
> SIGPIPE once in a while. I had to remove the final "To dst..." output
> check because there is no output if the process dies with SIGPIPE.
Thanks for a clear write-up.
> This patch accepts the SIGPIPE exit as legitimate test exit.
It is not this patch that accepts such a failure as legit ;-).

"Accept such a death-with-sigpipe also as OK when we are expecting a failure", perhaps?

Show 13 quoted lines
> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index ec22c98..22a941b 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -1162,15 +1162,15 @@ do
>  		mk_empty shallow &&
>  		(
>  			cd shallow &&
> -			test_must_fail git fetch ../testrepo/.git $SHA1_3 &&
> -			test_must_fail git fetch ../testrepo/.git $SHA1_1 &&
> +			test_must_fail_or_sigpipe git fetch ../testrepo/.git $SHA1_3 &&
> +			test_must_fail_or_sigpipe git fetch ../testrepo/.git $SHA1_1 &&
>  			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
These I understand.
>  			git fetch ../testrepo/.git $SHA1_1 &&
>  			git cat-file commit $SHA1_1 &&
> -			test_must_fail git cat-file commit $SHA1_2 &&
> +			test_must_fail_or_sigpipe git cat-file commit $SHA1_2 &&

This I don't. Under what condition is it sane for this "cat-file commit" to fail with sigpipe?

>  			git fetch ../testrepo/.git $SHA1_2 &&
>  			git cat-file commit $SHA1_2 &&
> -			test_must_fail git fetch ../testrepo/.git $SHA1_3
> +			test_must_fail_or_sigpipe git fetch ../testrepo/.git $SHA1_3
And this I do.
Show 34 quoted lines
>  		)
>  	'
>  done
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 73e37a1..19a598e 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -600,6 +600,29 @@ test_must_fail () {
>  	return 0
>  }
>
> +# Similar to test_must_fail, but tolerates sigpipe signal, too.
> +
> +test_must_fail_or_sigpipe () {
> +	"$@"
> +	exit_code=$?
> +	if test $exit_code = 0; then
> +		echo >&2 "test_must_fail: command succeeded: $*"
> +		return 1
> +	elif test $exit_code -ne 141 && \
> +		 test $exit_code -gt 129 && \
> +		 test $exit_code -le 192; then
> +		echo >&2 "test_must_fail: died by signal: $*"
> +		return 1
> +	elif test $exit_code = 127; then
> +		echo >&2 "test_must_fail: command not found: $*"
> +		return 1
> +	elif test $exit_code = 126; then
> +		echo >&2 "test_must_fail: valgrind error: $*"
> +		return 1
> +	fi
> +	return 0
> +}
> +

When you are coming up with this patch, you must have checked the existing code around this area. Did you notice that 126 is handled differently between must_fail and might_fail? Can you explain and justify the difference?

One explanation I can think of is that the contributor who did eeb69131 (tests: notice valgrind error in test_must_fail, 2013-03-31) did not notice that we do essentially the same thing in might_fail and forgot to add it. And it is not the fault of that contributor, but the fault of the duplicated and poorly organized code back then.

Adding the third variant in the way this patch does is making things worse by inviting more mistakes.

How about doing something like the attached to consolidate the existing two into one, and then build this third one on top?

 t/test-lib-functions.sh | 41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)
Show changes to t/test-lib-functions.sh +21 −20
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index e8d3c0f..54497d6 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -560,22 +560,7 @@ test_line_count () {
 # the failure could be due to a segv.  We want a controlled failure.
 
 test_must_fail () {
-	"$@"
-	exit_code=$?
-	if test $exit_code = 0; then
-		echo >&2 "test_must_fail: command succeeded: $*"
-		return 1
-	elif test $exit_code -gt 129 && test $exit_code -le 192; then
-		echo >&2 "test_must_fail: died by signal: $*"
-		return 1
-	elif test $exit_code = 127; then
-		echo >&2 "test_must_fail: command not found: $*"
-		return 1
-	elif test $exit_code = 126; then
-		echo >&2 "test_must_fail: valgrind error: $*"
-		return 1
-	fi
-	return 0
+	test_failure_helper test_must_fail "$@"
 }
 
 # Similar to test_must_fail, but tolerates success, too.  This is
@@ -590,13 +575,29 @@ test_must_fail () {
 # because we want to notice if it fails due to segv.
 
 test_might_fail () {
+	test_failure_helper test_might_fail "$@"
+}
+
+test_failure_helper () {
+	test_expect=$1
+	shift
 	"$@"
 	exit_code=$?
-	if test $exit_code -gt 129 && test $exit_code -le 192; then
-		echo >&2 "test_might_fail: died by signal: $*"
+	if test $test_expect != test_might_fail && test $exit_code = 0
+	then
+		echo >&2 "$test_expect: command succeeded: $*"
+		return 1
+	elif test $exit_code -gt 129 && test $exit_code -le 192
+	then
+		echo >&2 "$test_expect: died by signal: $*"
+		return 1
+	elif test $exit_code = 127
+	then
+		echo >&2 "$test_expect: command not found: $*"
 		return 1
-	elif test $exit_code = 127; then
-		echo >&2 "test_might_fail: command not found: $*"
+	elif test $exit_code = 126
+	then
+		echo >&2 "$test_expect: valgrind error: $*"
 		return 1
 	fi
 	return 0
Junio C Hamano· Nov 6, 2015, 18:49 UTC · re: Junio C Hamano · lore

Re: [PATCH v4 1/4] add function test_must_fail_or_sigpipe and use it to fix flaky tests

Junio C Hamano <gitster@pobox.com> writes:
Show 5 quoted lines
> Adding the third variant in the way this patch does is making things
> worse by inviting more mistakes.
>
> How about doing something like the attached to consolidate the
> existing two into one, and then build this third one on top?

Actually, I think this other variant I came up with is cleaner and is more easily extensible. It already has the support for sigpipe, e.g. you can do

	test_must_fail ok=sigpipe git fetch ...
Of course, you can do
	test_must_fail_or_sigpipe () {
		test_must_fail ok=sigpipe "$@"
	}

if it is easier to read. I do not have a very strong opinion either way.

 t/test-lib-functions.sh | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)
Show changes to t/test-lib-functions.sh +17 −11
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index e8d3c0f..b732f87 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -560,11 +560,26 @@ test_line_count () {
 # the failure could be due to a segv.  We want a controlled failure.
 
 test_must_fail () {
+	case "$1" in
+	ok=*)
+		_test_ok=${1#ok=}
+		shift
+		;;
+	*)
+		_test_ok=
+		;;
+	esac
 	"$@"
 	exit_code=$?
-	if test $exit_code = 0; then
+	if ! case ",$_test_ok," in *,success,*) false;; esac &&
+	   test $exit_code = 0
+	then
 		echo >&2 "test_must_fail: command succeeded: $*"
 		return 1
+	elif ! case ",$_test_ok," in *,sigpipe,*) false;; esac &&
+	   test $exit_code = 141
+	then
+		return 0
 	elif test $exit_code -gt 129 && test $exit_code -le 192; then
 		echo >&2 "test_must_fail: died by signal: $*"
 		return 1
@@ -590,16 +605,7 @@ test_must_fail () {
 # because we want to notice if it fails due to segv.
 
 test_might_fail () {
-	"$@"
-	exit_code=$?
-	if test $exit_code -gt 129 && test $exit_code -le 192; then
-		echo >&2 "test_might_fail: died by signal: $*"
-		return 1
-	elif test $exit_code = 127; then
-		echo >&2 "test_might_fail: command not found: $*"
-		return 1
-	fi
-	return 0
+	test_must_fail ok=success "$@"
 }
 
 # Similar to test_must_fail and test_might_fail, but check that a
larsxschneider@gmail.com· Nov 6, 2015, 08:58 UTC · re: larsxschneider@gmail.com · lore

[PATCH v4 2/4] git-p4: add p4d timeout in tests

From: Lars Schneider <larsxschneider@gmail.com>

In rare cases p4d seems to hang. This watchdog will kill the p4d process after 300s in any case. That means each individual git p4 test needs to finish before 300s or it will fail.

Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
 t/lib-git-p4.sh | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)
Show changes to t/lib-git-p4.sh +19 −1
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 7548225..b1660f6 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -6,6 +6,10 @@
 # a subdirectory called "$git"
 TEST_NO_CREATE_REPO=NoThanks
 
+# Sometimes p4d seems to hang. Terminate the p4d process automatically after
+# the defined timeout in seconds.
+P4D_TIMEOUT=300
+
 . ./test-lib.sh
 
 if ! test_have_prereq PYTHON
@@ -81,6 +85,19 @@ start_p4d() {
 	# will be caught with the "kill -0" check below.
 	i=${P4D_START_PATIENCE:-300}
 	pid=$(cat "$pidfile")
+
+	timeout=$(($(date +%s) + $P4D_TIMEOUT))
+    while true
+	do
+		if test $(date +%s) -gt $timeout
+		then
+			kill -9 $pid
+			exit 1
+		fi
+		sleep 1
+	done &
+	watchdog_pid=$!
+
 	ready=
 	while test $i -gt 0
 	do
@@ -131,7 +148,8 @@ kill_p4d() {
 	done &&
 	# complain if it would not die
 	test_must_fail kill $pid >/dev/null 2>&1 &&
-	rm -rf "$db" "$cli" "$pidfile"
+	rm -rf "$db" "$cli" "$pidfile" &&
+	retry_until_fail kill -9 $watchdog_pid
 }
 
 cleanup_git() {

-- 
2.5.1
Eric Sunshine· Nov 6, 2015, 09:23 UTC · re: larsxschneider@gmail.com · lore

Re: [PATCH v4 2/4] git-p4: add p4d timeout in tests

On Fri, Nov 6, 2015 at 3:58 AM,  <larsxschneider@gmail.com> wrote:
Show 14 quoted lines
> In rare cases p4d seems to hang. This watchdog will kill the p4d
> process after 300s in any case. That means each individual git p4 test
> needs to finish before 300s or it will fail.
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
> @@ -81,6 +85,19 @@ start_p4d() {
>         # will be caught with the "kill -0" check below.
>         i=${P4D_START_PATIENCE:-300}
>         pid=$(cat "$pidfile")
> +
> +       timeout=$(($(date +%s) + $P4D_TIMEOUT))
> +    while true
The 'while' line is incorrectly indented with spaces rather than tabs.
> +       do
> +               if test $(date +%s) -gt $timeout

I don't know how portable you intend this to be, but 'date +%s' is not universally supported (it's missing on Solaris, for instance, and perhaps AIX). For 6a9d16a (filter-branch: add passed/remaining seconds on progress, 2015-09-07), we ultimately settled upon detecting +%s support dynamically:

    if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'
        # it's supported
    fi

Perhaps you'd want to detect this via a lazy prerequisite and skip this if not supported?

Show 23 quoted lines
> +               then
> +                       kill -9 $pid
> +                       exit 1
> +               fi
> +               sleep 1
> +       done &
> +       watchdog_pid=$!
> +
>         ready=
>         while test $i -gt 0
>         do
> @@ -131,7 +148,8 @@ kill_p4d() {
>         done &&
>         # complain if it would not die
>         test_must_fail kill $pid >/dev/null 2>&1 &&
> -       rm -rf "$db" "$cli" "$pidfile"
> +       rm -rf "$db" "$cli" "$pidfile" &&
> +       retry_until_fail kill -9 $watchdog_pid
>  }
>
>  cleanup_git() {
> --
> 2.5.1
Lars Schneider· Nov 6, 2015, 13:20 UTC · re: Eric Sunshine · lore

Re: [PATCH v4 2/4] git-p4: add p4d timeout in tests

Show 19 quoted lines
> On 06 Nov 2015, at 10:23, Eric Sunshine <sunshine@sunshineco.com> wrote:
> 
> On Fri, Nov 6, 2015 at 3:58 AM,  <larsxschneider@gmail.com> wrote:
>> In rare cases p4d seems to hang. This watchdog will kill the p4d
>> process after 300s in any case. That means each individual git p4 test
>> needs to finish before 300s or it will fail.
>> 
>> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
>> ---
>> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
>> @@ -81,6 +85,19 @@ start_p4d() {
>>        # will be caught with the "kill -0" check below.
>>        i=${P4D_START_PATIENCE:-300}
>>        pid=$(cat "$pidfile")
>> +
>> +       timeout=$(($(date +%s) + $P4D_TIMEOUT))
>> +    while true
> 
> The 'while' line is incorrectly indented with spaces rather than tabs.'
Oh, you're right. One more thing that I will add to my pre mailing list submit check script :-)
Show 16 quoted lines
> 
>> +       do
>> +               if test $(date +%s) -gt $timeout
> 
> I don't know how portable you intend this to be, but 'date +%s' is not
> universally supported (it's missing on Solaris, for instance, and
> perhaps AIX). For 6a9d16a (filter-branch: add passed/remaining seconds
> on progress, 2015-09-07), we ultimately settled upon detecting +%s
> support dynamically:
> 
>    if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'
>        # it's supported
>    fi
> 
> Perhaps you'd want to detect this via a lazy prerequisite and skip
> this if not supported?
AFAIK 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!

Thanks, Lars

[1] https://www.perforce.com/perforce/doc.current/user/relnotes.txt
larsxschneider@gmail.com· Nov 6, 2015, 08:58 UTC · re: larsxschneider@gmail.com · lore

[PATCH v4 3/4] git-p4: retry kill/cleanup operations in tests with timeout

From: Lars Schneider <larsxschneider@gmail.com>

In rare cases kill/cleanup operations in tests fail. Retry these operations with a timeout to make the test less flaky.

Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
 t/lib-git-p4.sh | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)
Show changes to t/lib-git-p4.sh +23 −8
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index b1660f6..9043b51 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -10,6 +10,10 @@ TEST_NO_CREATE_REPO=NoThanks
 # the defined timeout in seconds.
 P4D_TIMEOUT=300
 
+# Some operations require multiple attempts to be successful. Define
+# here the maximal retry timeout in seconds.
+RETRY_TIMEOUT=60
+
 . ./test-lib.sh
 
 if ! test_have_prereq PYTHON
@@ -138,14 +142,24 @@ p4_add_user() {
 	EOF
 }
 
+retry_until_success() {
+    timeout=$(($(date +%s) + $RETRY_TIMEOUT))
+    until "$@" 2>/dev/null || test $(date +%s) -gt $timeout
+    do :
+    done
+}
+
+retry_until_fail() {
+    timeout=$(($(date +%s) + $RETRY_TIMEOUT))
+    until ! "$@" 2>/dev/null || test $(date +%s) -gt $timeout
+    do :
+    done
+}
+
 kill_p4d() {
 	pid=$(cat "$pidfile")
-	# it had better exist for the first kill
-	kill $pid &&
-	for i in 1 2 3 4 5 ; do
-		kill $pid >/dev/null 2>&1 || break
-		sleep 1
-	done &&
+	retry_until_fail kill $pid
+	retry_until_fail kill -9 $pid
 	# complain if it would not die
 	test_must_fail kill $pid >/dev/null 2>&1 &&
 	rm -rf "$db" "$cli" "$pidfile" &&
@@ -153,8 +167,9 @@ kill_p4d() {
 }
 
 cleanup_git() {
-	rm -rf "$git" &&
-	mkdir "$git"
+	retry_until_success rm -r "$git"
+	test_must_fail test -d "$git" &&
+	retry_until_success mkdir "$git"
 }
 
 marshal_dump() {

-- 
2.5.1
Eric Sunshine· Nov 6, 2015, 09:28 UTC · re: larsxschneider@gmail.com · lore

Re: [PATCH v4 3/4] git-p4: retry kill/cleanup operations in tests with timeout

On Fri, Nov 6, 2015 at 3:58 AM,  <larsxschneider@gmail.com> wrote:
Show 19 quoted lines
> In rare cases kill/cleanup operations in tests fail. Retry these
> operations with a timeout to make the test less flaky.
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
> +retry_until_success() {
> +    timeout=$(($(date +%s) + $RETRY_TIMEOUT))
> +    until "$@" 2>/dev/null || test $(date +%s) -gt $timeout
> +    do :
> +    done
> +}
> +
> +retry_until_fail() {
> +    timeout=$(($(date +%s) + $RETRY_TIMEOUT))
> +    until ! "$@" 2>/dev/null || test $(date +%s) -gt $timeout
> +    do :
> +    done
> +}

I'm confused by this. Patch 2/4 was already calling retry_until_fail(), but it's introduction seems to be here in patch 3/4. Am I missing something obvious?

Show 27 quoted lines
>  kill_p4d() {
>         pid=$(cat "$pidfile")
> -       # it had better exist for the first kill
> -       kill $pid &&
> -       for i in 1 2 3 4 5 ; do
> -               kill $pid >/dev/null 2>&1 || break
> -               sleep 1
> -       done &&
> +       retry_until_fail kill $pid
> +       retry_until_fail kill -9 $pid
>         # complain if it would not die
>         test_must_fail kill $pid >/dev/null 2>&1 &&
>         rm -rf "$db" "$cli" "$pidfile" &&
> @@ -153,8 +167,9 @@ kill_p4d() {
>  }
>
>  cleanup_git() {
> -       rm -rf "$git" &&
> -       mkdir "$git"
> +       retry_until_success rm -r "$git"
> +       test_must_fail test -d "$git" &&
> +       retry_until_success mkdir "$git"
>  }
>
>  marshal_dump() {
> --
> 2.5.1
Lars Schneider· Nov 6, 2015, 09:47 UTC · re: Eric Sunshine · lore

Re: [PATCH v4 3/4] git-p4: retry kill/cleanup operations in tests with timeout

Show 26 quoted lines
> On 06 Nov 2015, at 10:28, Eric Sunshine <sunshine@sunshineco.com> wrote:
> 
> On Fri, Nov 6, 2015 at 3:58 AM,  <larsxschneider@gmail.com> wrote:
>> In rare cases kill/cleanup operations in tests fail. Retry these
>> operations with a timeout to make the test less flaky.
>> 
>> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
>> ---
>> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
>> +retry_until_success() {
>> +    timeout=$(($(date +%s) + $RETRY_TIMEOUT))
>> +    until "$@" 2>/dev/null || test $(date +%s) -gt $timeout
>> +    do :
>> +    done
>> +}
>> +
>> +retry_until_fail() {
>> +    timeout=$(($(date +%s) + $RETRY_TIMEOUT))
>> +    until ! "$@" 2>/dev/null || test $(date +%s) -gt $timeout
>> +    do :
>> +    done
>> +}
> 
> I'm confused by this. Patch 2/4 was already calling
> retry_until_fail(), but it's introduction seems to be here in patch
> 3/4. Am I missing something obvious?
No, my fault. I reordered the commits and forgot about this. I will fix the order in the next roll.

Thanks, Lars

Lars Schneider· Nov 6, 2015, 13:19 UTC · re: Eric Sunshine · lore

Re: [PATCH v4 3/4] git-p4: retry kill/cleanup operations in tests with timeout

Show 26 quoted lines
> On 06 Nov 2015, at 10:28, Eric Sunshine <sunshine@sunshineco.com> wrote:
> 
> On Fri, Nov 6, 2015 at 3:58 AM,  <larsxschneider@gmail.com> wrote:
>> In rare cases kill/cleanup operations in tests fail. Retry these
>> operations with a timeout to make the test less flaky.
>> 
>> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
>> ---
>> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
>> +retry_until_success() {
>> +    timeout=$(($(date +%s) + $RETRY_TIMEOUT))
>> +    until "$@" 2>/dev/null || test $(date +%s) -gt $timeout
>> +    do :
>> +    done
>> +}
>> +
>> +retry_until_fail() {
>> +    timeout=$(($(date +%s) + $RETRY_TIMEOUT))
>> +    until ! "$@" 2>/dev/null || test $(date +%s) -gt $timeout
>> +    do :
>> +    done
>> +}
> 
> I'm confused by this. Patch 2/4 was already calling
> retry_until_fail(), but it's introduction seems to be here in patch
> 3/4. Am I missing something obvious?
No, my fault. I reordered the commits and forgot about this. I will fix the order in the next roll.

Thanks, Lars

larsxschneider@gmail.com· Nov 6, 2015, 08:58 UTC · re: larsxschneider@gmail.com · lore

[PATCH v4 4/4] Add Travis CI support

From: Lars Schneider <larsxschneider@gmail.com>

The tests are currently executed on "Ubuntu 12.04 LTS Server Edition 64 bit" and on "OS X Mavericks" using gcc and clang.

Perforce and Git-LFS are installed and therefore available for the respective tests.

Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
 .travis.yml | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 131 insertions(+)
 create mode 100644 .travis.yml
Show changes to .travis.yml +131 −0
diff --git a/.travis.yml b/.travis.yml
new file mode 100644
index 0000000..61c70fa
--- /dev/null
+++ b/.travis.yml
@@ -0,0 +1,131 @@
+language: c
+
+os:
+  - linux
+  - osx
+
+compiler:
+  - clang
+  - gcc
+
+addons:
+  apt:
+    packages:
+    - language-pack-is
+
+env:
+  global:
+    - P4_VERSION="15.1"
+    - GIT_LFS_VERSION="1.0.2"
+    - DEFAULT_TEST_TARGET=prove
+    - GIT_PROVE_OPTS="--timer --jobs 3"
+    - GIT_TEST_OPTS="--verbose --tee"
+    - GETTEXT_ISO_LOCALE=YesPlease
+    - GETTEXT_LOCALE=YesPlease
+    # - GETTEXT_POISON=YesPlease
+    - GIT_TEST_CHAIN_LINT=YesPlease
+    - GIT_TEST_CLONE_2GB=YesPlease
+    # - GIT_TEST_LONG=YesPlease
+  matrix:
+    -
+      # NO_ICONV=YesPlease
+    - >
+      NO_CURL=YesPlease
+      NO_D_INO_IN_DIRENT=YesPlease
+      NO_DEFLATE_BOUND=YesPlease
+      NO_EXPAT=YesPlease
+      NO_GECOS_IN_PWENT=YesPlease
+      NO_GETTEXT=YesPlease
+      NO_HMAC_CTX_CLEANUP=YesPlease
+      NO_HSTRERROR=YesPlease
+      NO_INET_NTOP=YesPlease
+      NO_INET_PTON=YesPlease
+      NO_INITGROUPS=YesPlease
+      NO_INTTYPES_H=YesPlease
+      NO_IPV6=YesPlease
+      NO_IPV6=YesPlease
+      NO_LIBGEN_H=YesPlease
+      NO_MEMMEM=YesPlease
+      NO_MKDTEMP=YesPlease
+      NO_MKSTEMPS=YesPlease
+      NO_MMAP=YesPlease
+      NO_NSEC=YesPlease
+      NO_OPENSSL=YesPlease
+      NO_PERL=YesPlease
+      NO_PTHREADS=YesPlease
+      NO_REGEX=YesPlease
+      NO_SETENV=YesPlease
+      NO_SETITIMER=YesPlease
+      NO_SOCKADDR_STORAGE=YesPlease
+      NO_STRCASESTR=YesPlease
+      NO_STRLCPY=YesPlease
+      NO_STRTOUMAX=YesPlease
+      NO_STRUCT_ITIMERVAL=YesPlease
+      NO_SYMLINK_HEAD=YesPlease
+      NO_SYS_POLL_H=YesPlease
+      NO_SYS_SELECT_H=YesPlease
+      NO_UINTMAX_T=YesPlease
+      NO_UNSETENV=YesPlease
+
+before_install:
+  - >
+    case "${TRAVIS_OS_NAME:-linux}" in
+    linux)
+      mkdir --parents custom/p4
+      pushd custom/p4
+        wget --quiet http://filehost.perforce.com/perforce/r$P4_VERSION/bin.linux26x86_64/p4d
+        wget --quiet http://filehost.perforce.com/perforce/r$P4_VERSION/bin.linux26x86_64/p4
+        chmod u+x p4d
+        chmod u+x p4
+        export PATH="$(pwd):$PATH"
+      popd
+      mkdir --parents custom/git-lfs
+      pushd custom/git-lfs
+        wget --quiet https://github.com/github/git-lfs/releases/download/v$GIT_LFS_VERSION/git-lfs-linux-amd64-$GIT_LFS_VERSION.tar.gz
+        tar --extract --gunzip --file "git-lfs-linux-amd64-$GIT_LFS_VERSION.tar.gz"
+        cp git-lfs-$GIT_LFS_VERSION/git-lfs .
+        export PATH="$(pwd):$PATH"
+      popd
+      ;;
+    osx)
+      brew_force_set_latest_binary_hash () {
+        FORMULA=$1
+        SHA=$(brew fetch --force $FORMULA 2>&1 | grep ^SHA256: | cut -d ' ' -f 2)
+        sed -E -i.bak "s/sha256 \"[0-9a-f]{64}\"/sha256 \"$SHA\"/g" \
+          /usr/local/Library/Taps/homebrew/homebrew-binary/$FORMULA.rb
+      }
+      brew update --quiet
+      brew tap homebrew/binary --quiet
+      brew_force_set_latest_binary_hash perforce
+      brew_force_set_latest_binary_hash perforce-server
+      brew install git-lfs perforce-server perforce
+      ;;
+    esac;
+    echo "$(tput setaf 6)Perfoce Server Version$(tput sgr0)";
+    p4d -V | grep Rev.;
+    echo "$(tput setaf 6)Perfoce Client Version$(tput sgr0)";
+    p4 -V | grep Rev.;
+    echo "$(tput setaf 6)Git-LFS Version$(tput sgr0)";
+    git-lfs version;
+
+before_script: make configure && ./configure && make --jobs=2
+
+script: make --quiet test
+
+after_failure:
+  - >
+    : '<-- Click here to see detailed test output!                                                        ';
+    for TEST_EXIT in t/test-results/*.exit;
+    do
+      if [ "$(cat "$TEST_EXIT")" != "0" ];
+      then
+        TEST_OUT="${TEST_EXIT%exit}out";
+        echo "------------------------------------------------------------------------";
+        echo "$(tput setaf 1)${TEST_OUT}...$(tput sgr0)";
+        echo "------------------------------------------------------------------------";
+        cat "${TEST_OUT}";
+      fi;
+    done;
+
+notifications:
+  email: false

-- 
2.5.1
Eric Sunshine· Nov 6, 2015, 09:56 UTC · re: larsxschneider@gmail.com · lore

Re: [PATCH v4 4/4] Add Travis CI support

On Fri, Nov 6, 2015 at 3:58 AM,  <larsxschneider@gmail.com> wrote:
Show 50 quoted lines
> The tests are currently executed on "Ubuntu 12.04 LTS Server Edition
> 64 bit" and on "OS X Mavericks" using gcc and clang.
>
> Perforce and Git-LFS are installed and therefore available for the
> respective tests.
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
> diff --git a/.travis.yml b/.travis.yml
> @@ -0,0 +1,131 @@
> +  matrix:
> +    -
> +      # NO_ICONV=YesPlease
> +    - >
> +      NO_CURL=YesPlease
> +      NO_D_INO_IN_DIRENT=YesPlease
> +      NO_DEFLATE_BOUND=YesPlease
> +      NO_EXPAT=YesPlease
> +      NO_GECOS_IN_PWENT=YesPlease
> +      NO_GETTEXT=YesPlease
> +      NO_HMAC_CTX_CLEANUP=YesPlease
> +      NO_HSTRERROR=YesPlease
> +      NO_INET_NTOP=YesPlease
> +      NO_INET_PTON=YesPlease
> +      NO_INITGROUPS=YesPlease
> +      NO_INTTYPES_H=YesPlease
> +      NO_IPV6=YesPlease
> +      NO_IPV6=YesPlease
> +      NO_LIBGEN_H=YesPlease
> +      NO_MEMMEM=YesPlease
> +      NO_MKDTEMP=YesPlease
> +      NO_MKSTEMPS=YesPlease
> +      NO_MMAP=YesPlease
> +      NO_NSEC=YesPlease
> +      NO_OPENSSL=YesPlease
> +      NO_PERL=YesPlease
> +      NO_PTHREADS=YesPlease
> +      NO_REGEX=YesPlease
> +      NO_SETENV=YesPlease
> +      NO_SETITIMER=YesPlease
> +      NO_SOCKADDR_STORAGE=YesPlease
> +      NO_STRCASESTR=YesPlease
> +      NO_STRLCPY=YesPlease
> +      NO_STRTOUMAX=YesPlease
> +      NO_STRUCT_ITIMERVAL=YesPlease
> +      NO_SYMLINK_HEAD=YesPlease
> +      NO_SYS_POLL_H=YesPlease
> +      NO_SYS_SELECT_H=YesPlease
> +      NO_UINTMAX_T=YesPlease
> +      NO_UNSETENV=YesPlease

Hmm, aren't you losing test coverage by disabling some of these? Is that desirable for continuous integration testing? Am I missing something?

Lars Schneider· Nov 6, 2015, 13:18 UTC · re: Eric Sunshine · lore

Re: [PATCH v4 4/4] Add Travis CI support

Show 57 quoted lines
> On 06 Nov 2015, at 10:56, Eric Sunshine <sunshine@sunshineco.com> wrote:
> 
> On Fri, Nov 6, 2015 at 3:58 AM,  <larsxschneider@gmail.com> wrote:
>> The tests are currently executed on "Ubuntu 12.04 LTS Server Edition
>> 64 bit" and on "OS X Mavericks" using gcc and clang.
>> 
>> Perforce and Git-LFS are installed and therefore available for the
>> respective tests.
>> 
>> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
>> ---
>> diff --git a/.travis.yml b/.travis.yml
>> @@ -0,0 +1,131 @@
>> +  matrix:
>> +    -
>> +      # NO_ICONV=YesPlease
>> +    - >
>> +      NO_CURL=YesPlease
>> +      NO_D_INO_IN_DIRENT=YesPlease
>> +      NO_DEFLATE_BOUND=YesPlease
>> +      NO_EXPAT=YesPlease
>> +      NO_GECOS_IN_PWENT=YesPlease
>> +      NO_GETTEXT=YesPlease
>> +      NO_HMAC_CTX_CLEANUP=YesPlease
>> +      NO_HSTRERROR=YesPlease
>> +      NO_INET_NTOP=YesPlease
>> +      NO_INET_PTON=YesPlease
>> +      NO_INITGROUPS=YesPlease
>> +      NO_INTTYPES_H=YesPlease
>> +      NO_IPV6=YesPlease
>> +      NO_IPV6=YesPlease
>> +      NO_LIBGEN_H=YesPlease
>> +      NO_MEMMEM=YesPlease
>> +      NO_MKDTEMP=YesPlease
>> +      NO_MKSTEMPS=YesPlease
>> +      NO_MMAP=YesPlease
>> +      NO_NSEC=YesPlease
>> +      NO_OPENSSL=YesPlease
>> +      NO_PERL=YesPlease
>> +      NO_PTHREADS=YesPlease
>> +      NO_REGEX=YesPlease
>> +      NO_SETENV=YesPlease
>> +      NO_SETITIMER=YesPlease
>> +      NO_SOCKADDR_STORAGE=YesPlease
>> +      NO_STRCASESTR=YesPlease
>> +      NO_STRLCPY=YesPlease
>> +      NO_STRTOUMAX=YesPlease
>> +      NO_STRUCT_ITIMERVAL=YesPlease
>> +      NO_SYMLINK_HEAD=YesPlease
>> +      NO_SYS_POLL_H=YesPlease
>> +      NO_SYS_SELECT_H=YesPlease
>> +      NO_UINTMAX_T=YesPlease
>> +      NO_UNSETENV=YesPlease
> 
> Hmm, aren't you losing test coverage by disabling some of these? Is
> that desirable for continuous integration testing? Am I missing
> something?
Per 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...").

You can see all 8 configurations ([linux, mac] * [clang, gcc] * [normal, NO_...]) here: https://travis-ci.org/larsxschneider/git/builds/89598194

Cheers, Lars

Sebastian Schuberth· Nov 6, 2015, 13:20 UTC · lore

Re: [PATCH v4 4/4] Add Travis CI support

On Fri, Nov 6, 2015 at 2:11 PM, Lars Schneider <larsxschneider@gmail.com> wrote:
Show 7 quoted lines
> Per 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...").
>
> You can see all 8 configurations ([linux, mac] * [clang, gcc] * [normal,
> NO_...]) here:
> https://travis-ci.org/larsxschneider/git/builds/89598194

Aren't these 8 configurations a bit too much? I see the total running time is about 2 hours. For my taste, this is way to much to give developers feedback about the status of their PR. It should be something < 30 minutes.

IMO, the purpose of the Travis CI configuration mainly is to 1) save developers work by not requiring to build manually, 2) build on other platforms than the developer has access to. I doubt that the average developer manually builds anything close to these 8 configurations before we had this job, so I wouldn't make it a requirement for Travis to do much more than a developer could / would to manually.

On the other hand, I see the point in letting a CI system test more configurations than manually feasible. But that should be done as part of a different job then. E.g. we could have a "fast" PR validation job, and a "slow" nightly build job.

-- 
Sebastian Schuberth
Lars Schneider· Nov 6, 2015, 13:28 UTC · re: Sebastian Schuberth · lore

Re: [PATCH v4 4/4] Add Travis CI support

Show 29 quoted lines
> On 06 Nov 2015, at 14:20, Sebastian Schuberth <sschuberth@gmail.com> wrote:
> 
> On Fri, Nov 6, 2015 at 2:11 PM, Lars Schneider <larsxschneider@gmail.com> wrote:
> 
>> Per 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...").
>> 
>> You can see all 8 configurations ([linux, mac] * [clang, gcc] * [normal,
>> NO_...]) here:
>> https://travis-ci.org/larsxschneider/git/builds/89598194
> 
> Aren't these 8 configurations a bit too much? I see the total running
> time is about 2 hours. For my taste, this is way to much to give
> developers feedback about the status of their PR. It should be
> something < 30 minutes.
> 
> IMO, the purpose of the Travis CI configuration mainly is to 1) save
> developers work by not requiring to build manually, 2) build on other
> platforms than the developer has access to. I doubt that the average
> developer manually builds anything close to these 8 configurations
> before we had this job, so I wouldn't make it a requirement for Travis
> to do much more than a developer could / would to manually.
> 
> On the other hand, I see the point in letting a CI system test more
> configurations than manually feasible. But that should be done as part
> of a different job then. E.g. we could have a "fast" PR validation
> job, and a "slow" nightly build job.
> 
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).
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).
What do you think?

Thanks, Lars

Sebastian Schuberth· Nov 6, 2015, 13:36 UTC · re: Lars Schneider · lore

Re: [PATCH v4 4/4] Add Travis CI support

On Fri, Nov 6, 2015 at 2:28 PM, Lars Schneider <larsxschneider@gmail.com> wrote:
> 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).

Are you sure about than? I mean, what sense does it make to show how long it *would* have taken *if* the builds were running serially? I can see that the longest of the jobs took 21 minutes, which is ok. But that does not mean that all jobs completed in within 21 minutes. It could be that not all jobs started at (about) the same time due to a lack of resources, and that the last job did not compete before the 2 hours were over because it only started to run 1 hours and 40 minutes befor ethe first job was started.

> 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).
>
> What do you think?
I think running different configuration per branch makes sense, yes.
-- 
Sebastian Schuberth
Lars Schneider· Nov 6, 2015, 13:55 UTC · re: Sebastian Schuberth · lore

Re: [PATCH v4 4/4] Add Travis CI support

Show 14 quoted lines
> On 06 Nov 2015, at 14:36, Sebastian Schuberth <sschuberth@gmail.com> wrote:
> 
> On Fri, Nov 6, 2015 at 2:28 PM, Lars Schneider <larsxschneider@gmail.com> wrote:
> 
>> 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).
> 
> Are you sure about than? I mean, what sense does it make to show how
> long it *would* have taken *if* the builds were running serially? I
> can see that the longest of the jobs took 21 minutes, which is ok. But
> that does not mean that all jobs completed in within 21 minutes. It
> could be that not all jobs started at (about) the same time due to a
> lack of resources, and that the last job did not compete before the 2
> hours were over because it only started to run 1 hours and 40 minutes
> befor ethe first job was started.
I am fairly certain about this.

See, here is the first configuration and the first test case of a job: https://travis-ci.org/larsxschneider/git/jobs/89598195 [08:21:06] t0002-gitfile.sh

Here is the last configuration and the last test case of the same job: [08:51:22] t9903-bash-prompt.sh

~30 min for all 8 configurations. You can enable Travis CI for you GitHub account and try it easily yourself :-)
Show 6 quoted lines
> 
>> 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).
>> 
>> What do you think?
> 
> I think running different configuration per branch makes sense, yes.
If the list decides to accept this patch. Do you think that would be a necessary requirement for the first iteration?

Thanks, Lars

Sebastian Schuberth· Nov 6, 2015, 13:57 UTC · re: Lars Schneider · lore

Re: [PATCH v4 4/4] Add Travis CI support

On Fri, Nov 6, 2015 at 2:55 PM, Lars Schneider <larsxschneider@gmail.com> wrote:
>> I think running different configuration per branch makes sense, yes.
>
> If the list decides to accept this patch. Do you think that would be a necessary requirement for the first iteration?

No. I think this could be addressed later as an improvements. To me it's more important to finally get *some* sane Travis CI configuration in.

-- 
Sebastian Schuberth
Lars Schneider· Nov 6, 2015, 14:08 UTC · re: Sebastian Schuberth · lore

Re: [PATCH v4 4/4] Add Travis CI support

Show 11 quoted lines
> On 06 Nov 2015, at 14:57, Sebastian Schuberth <sschuberth@gmail.com> wrote:
> 
> On Fri, Nov 6, 2015 at 2:55 PM, Lars Schneider <larsxschneider@gmail.com> wrote:
> 
>>> I think running different configuration per branch makes sense, yes.
>> 
>> If the list decides to accept this patch. Do you think that would be a necessary requirement for the first iteration?
> 
> No. I think this could be addressed later as an improvements. To me
> it's more important to finally get *some* sane Travis CI configuration
> in.

True. 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: https://s3.amazonaws.com/archive.travis-ci.org/jobs/89603763/log.txt

I also see a weird "prove Tests our of sequence" error one in a while: https://s3.amazonaws.com/archive.travis-ci.org/jobs/89603770/log.txt

Does anyone have an idea what could cause this?

Thanks, Lars

← back to recent threads