Re: [PATCH v6 0/6] Add Travis CI support
On 19 Nov 2015, at 15:14, Jeff King <peff@peff.net> wrote:
Show 15 quoted lines
> On Thu, Nov 19, 2015 at 09:58:05AM +0100, larsxschneider@gmail.com wrote:
>
>> From: Lars Schneider <larsxschneider@gmail.com>
>>
>> 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.
Show 10 quoted lines
>
>> 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.
Show 6 quoted lines
>
>> 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 <peff@peff.net>" ? Bare with me, I am still learning ;-)
Show 8 quoted lines
>
>> 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.
Show 5 quoted lines
>
>> Add Travis CI support
>
> I'll leave some comments directly in response to this one.
>
Thanks for taking the time to review this!
- Lars
Lars Schneider <larsxschneider@gmail.com>