On 12/13/2016 01:07 PM, Fraser Tweedale wrote:
On Tue, Dec 13, 2016 at 09:41:40AM +0100, Martin Babinsky wrote:
Hi list,
https://github.com/freeipa/freeipa/pull/177 was recently merged despite
causing nearly half of the tests in our Travis CI gating to fail. This broke
Travis CI for all other PR that were rebased after this merge, causing false
negative errors everywhere.
Fraser reverted the offending commits in
https://github.com/freeipa/freeipa/pull/329 which restored Travis to
original state (never mind PEP8 errors they were in the original code
already).
Regarding this issues I have two questions:
a)
should I merge https://github.com/freeipa/freeipa/pull/329 and thus revert
the breakage in order to unblock other contributors? Given the current
traffic I think it is sufficient to wait for us to investigate and produce a
fix. If not, please scream loudly.
Definitely not, I am using this PR as a playground to poke the CI in
various ways. Also, proper fix would be best :) See also my other
mail (I would have replied here but fetchmail was not fetching mail
and I missed it until just now >_<).
b)
what can we improve to make the results of CI more visible to contributors?
I think that I should sit down with Martin 2 and investigate the possibility
to send notifications about negative CI results (sufficient IMO) to the
mailing list.
Or the commit author.
It would be *great* if the test job, in event of failure, would
collect all the obvious logs and dump them somewhere as artifacts.
That would be great but AFAIK Travis CI does support only archiving
artifacts into a running AWS instance[1] which we do not have and would
have to buy, run and maintain ourselves. For now we have to inspect the
log dumps generated in the job output.
In the meanwhile I would like to ask all reviewers to carefully check the
output of failed Travis CI runs. If the job fails, you will see the results
at the very end of the log. There are two sections: PEP8 errors and test
output. You can expand both of them to see what went wrong and report it to
the PR author if necessary.
The reviewer and author can then use the very same tool used in CI [1] to
reproduce the failures locally. Using '--no-cleanup' option during the run
[2] leaves behind a running container which you can attach to and
investigate further.
[1] https://github.com/freeipa/ipa-docker-test-runner
[2] https://github.com/freeipa/ipa-docker-test-runner/blob/master/README.md
If you have any additional questions/suggestions about Travis feel free to
contact me.
--
Martin^3 Babinsky
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[1] https://docs.travis-ci.com/user/uploading-artifacts/
--
Martin^3 Babinsky
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
