Re: [PATCH v6 0/6] Add Travis CI support

[Luke Diamand]

On 19/11/15 08:58, larsxschnei...@gmail.com wrote:

From: Lars Schneider 

diff to v5:
* check if PID file still exists on P4D cleanup (thanks Luke)
* fix space/tab formatting error
* add sleep to timeout loops (thanks Luke)
* replace 'date +%s' with platform independent Python function (thanks Eric and 
Luke)


The first three of these (which fix the git-p4 tests) all look good to 
me. With these changes, running the t98* tests in parallel now works, 
which it did not used to do. The changes are not pretty but I think 
that's inevitable.


Ack from me.

Luke

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 0/6] Add Travis CI support

[Luke Diamand]

On 20 November 2015 at 08:46, Lars Schneider  wrote:
>
> On 19 Nov 2015, at 15:14, Jeff King  wrote:
>
>
>>
>>>  git-p4: retry kill/cleanup operations in tests with timeout
>>>  git-p4: add p4d timeout in tests
>>>  git-p4: add trap to kill p4d on test exit
>>
>> These are all fairly gross, and I don't have p4d to test with myself.
>> But if we assume they're all necessary, I suppose it's the best we can
>> do.
>
> Unfortunately I think they are necessary. However, if someone finds a better 
> way for stable p4d tests then I would be happy to see them go away, again.

I think that's just how p4d is I'm afraid. It doesn't like being
stopped and started quickly (I guess it's not a normal use-case for
most p4 users). I've made various unsuccessful attempts in the past to
make these tests work reliably, and Lars' changes are far better than
anything I ever managed.

Luke
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 0/6] Add Travis CI support

[Lars Schneider]

On 19 Nov 2015, at 15:14, Jeff King  wrote:

> On Thu, Nov 19, 2015 at 09:58:05AM +0100, larsxschnei...@gmail.com wrote:
> 
>> From: Lars Schneider 
>> 
>> diff to v5:
>> * check if PID file still exists on P4D cleanup (thanks Luke)
>> * fix space/tab formatting error
>> * add sleep to timeout loops (thanks Luke)
>> * replace 'date +%s' with platform independent Python function (thanks Eric 
>> and Luke)
>> 
>> With the patches of this series the Travis CI test stability increases.
>> However, as I am "stress testing" the Travis CI infrastructure you can
>> see that it is not perfect: https://travis-ci.org/larsxschneider/git/builds
> 
> I peeked at a few, and it looks like just p4 tests failing now?

Yes, in particular t9810-git-p4-rcs.sh and t9816-git-p4-locked.sh. I would 
probably disable these test in Travis CI until I've found a way to make it 
stable.

> 
>> Nevertheless, I believe that Travis CI integration has still value as
>> contributors can test their patches easily on Linux and OS X before
>> posting them.
>> 
>> @junio / @peff: Do you consider merging this?
> 
> I think I'd prefer to split it into 3 separate topics (de-flaking
> test_must_fail, p4 test improvements, and the Travis file). Then they
> can proceed independently. I can take care of that split when applying.

Sounds good to me!

> 
>> Lars Schneider (6):
>>  implement test_might_fail using a refactored test_must_fail
> 
> You mentioned in the v5 cover that this one was from Junio. Should it be
> "From: Junio ..." in the pseudo-header?

Yes, this one was from Junio with a minor fix from my end if I recall 
correctly. What do you mean by "pseudo-header"? The "email-header" in the patch 
file? 

> 
>>  add "ok=sigpipe" to test_must_fail and use it to fix flaky tests
> 
> Looks OK.

"Looks OK" means I can/should add "Acked-by: Jeff King " ? Bare 
with me, I am still learning ;-)

> 
>>  git-p4: retry kill/cleanup operations in tests with timeout
>>  git-p4: add p4d timeout in tests
>>  git-p4: add trap to kill p4d on test exit
> 
> These are all fairly gross, and I don't have p4d to test with myself.
> But if we assume they're all necessary, I suppose it's the best we can
> do.

Unfortunately I think they are necessary. However, if someone finds a better 
way for stable p4d tests then I would be happy to see them go away, again.

> 
>>  Add Travis CI support
> 
> I'll leave some comments directly in response to this one.
> 

Thanks for taking the time to review this!

- Lars

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 0/6] Add Travis CI support

[Jeff King]

On Thu, Nov 19, 2015 at 09:58:05AM +0100, larsxschnei...@gmail.com wrote:

> From: Lars Schneider 
> 
> diff to v5:
> * check if PID file still exists on P4D cleanup (thanks Luke)
> * fix space/tab formatting error
> * add sleep to timeout loops (thanks Luke)
> * replace 'date +%s' with platform independent Python function (thanks Eric 
> and Luke)
> 
> With the patches of this series the Travis CI test stability increases.
> However, as I am "stress testing" the Travis CI infrastructure you can
> see that it is not perfect: https://travis-ci.org/larsxschneider/git/builds

I peeked at a few, and it looks like just p4 tests failing now?

> Nevertheless, I believe that Travis CI integration has still value as
> contributors can test their patches easily on Linux and OS X before
> posting them.
> 
> @junio / @peff: Do you consider merging this?

I think I'd prefer to split it into 3 separate topics (de-flaking
test_must_fail, p4 test improvements, and the Travis file). Then they
can proceed independently. I can take care of that split when applying.

> Lars Schneider (6):
>   implement test_might_fail using a refactored test_must_fail

You mentioned in the v5 cover that this one was from Junio. Should it be
"From: Junio ..." in the pseudo-header?

>   add "ok=sigpipe" to test_must_fail and use it to fix flaky tests

Looks OK.

>   git-p4: retry kill/cleanup operations in tests with timeout
>   git-p4: add p4d timeout in tests
>   git-p4: add trap to kill p4d on test exit

These are all fairly gross, and I don't have p4d to test with myself.
But if we assume they're all necessary, I suppose it's the best we can
do.

>   Add Travis CI support

I'll leave some comments directly in response to this one.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html