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

On 19 Nov 2015, at 15:14, Jeff King [off-list ref] wrote:

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

quoted

From: Lars Schneider <redacted>

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.

quoted

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!

quoted

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? 

quoted

 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 [off-list ref]" ? Bare with me, I am still learning ;-)

quoted

 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.

quoted

 Add Travis CI support

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

Thanks for taking the time to review this!

- Lars