FAQ
Hi!

As many of us know, we have a setup on Travis CI which automatically
runs unit tests for our active branches. This is very convenient for
evaluating pull request and status of the branches and keeping us from
making changes that break something without us noticing. However,
there's a problem with this setup, and the problem is that CI build is
never kept in the green for more than a a few days, the "normal" state
of it is always failing. I think this situation is not normal and,
frankly, embarrassing for the project, and propose to institute a policy
to change it.
The start would be recognizing that the goal is to keep the CI tests
green. Once they are green, and somebody commits a change that makes
them fail, we have the following options (note I'm not advocating for
now any of them, just enumerating them) to fix it:

1. Revert the change immediately, and ask the submitter to fix the pull
(pulls can be tested too on CI too) and re-submit it when it's green.

2. Wait for the change developer for a reasonable short time to fix the
tests, if that does not happen, revert the change.

3. Put the failing tests into XFAIL until they are fixed by somebody.

4. Have somebody - e.g. RM for the branch - to modify the failing test
to reflect the new results.

These options, of course, can be used in combination and in
case-per-case basis, but I think we need some explicit attention to the
topic. I welcome everybody to propose suggestions about other options or
ways we could fix this situation going forward and would like to hear
the ideas about how we could make Travis CI tests useful and passing
successfully, thus ensuring better quality in the project. After the
initial discussion, I plan to write an RFC summarizing the proposals and
have it voted on and, hopefully, accepted, so we'd have official policy
on it, but fir now I'd also like to raise awareness of the issue and
solicit ideas and participation of the members of the project on this topic.

Thanks,
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

Search Discussions

  • Nikita Popov at Apr 5, 2014 at 8:19 pm

    On Sat, Apr 5, 2014 at 8:41 PM, Stas Malyshev wrote:

    Hi!

    As many of us know, we have a setup on Travis CI which automatically
    runs unit tests for our active branches. This is very convenient for
    evaluating pull request and status of the branches and keeping us from
    making changes that break something without us noticing. However,
    there's a problem with this setup, and the problem is that CI build is
    never kept in the green for more than a a few days, the "normal" state
    of it is always failing. I think this situation is not normal and,
    frankly, embarrassing for the project, and propose to institute a policy
    to change it.
    The start would be recognizing that the goal is to keep the CI tests
    green. Once they are green, and somebody commits a change that makes
    them fail, we have the following options (note I'm not advocating for
    now any of them, just enumerating them) to fix it:

    1. Revert the change immediately, and ask the submitter to fix the pull
    (pulls can be tested too on CI too) and re-submit it when it's green.

    2. Wait for the change developer for a reasonable short time to fix the
    tests, if that does not happen, revert the change.

    3. Put the failing tests into XFAIL until they are fixed by somebody.

    4. Have somebody - e.g. RM for the branch - to modify the failing test
    to reflect the new results.

    These options, of course, can be used in combination and in
    case-per-case basis, but I think we need some explicit attention to the
    topic. I welcome everybody to propose suggestions about other options or
    ways we could fix this situation going forward and would like to hear
    the ideas about how we could make Travis CI tests useful and passing
    successfully, thus ensuring better quality in the project. After the
    initial discussion, I plan to write an RFC summarizing the proposals and
    have it voted on and, hopefully, accepted, so we'd have official policy
    on it, but fir now I'd also like to raise awareness of the issue and
    solicit ideas and participation of the members of the project on this
    topic.
    It would be a good start to send a mail to the person introducing a CI
    build failure automatically. I think to the most part people are simply not
    aware that they introduce failures - it's not like anybody goes on travis
    half an hour after doing a commit to check whether everything works fine.
    Maybe bugging people about this is already enough to keep a green build
    most of the time?

    Nikita
  • Stas Malyshev at Apr 6, 2014 at 1:40 am
    Hi!
    It would be a good start to send a mail to the person introducing a CI
    build failure automatically. I think to the most part people are simply
    not aware that they introduce failures - it's not like anybody goes on
    travis half an hour after doing a commit to check whether everything
    works fine. Maybe bugging people about this is already enough to keep a
    green build most of the time?
    Travis seems to have a way to do this:
    http://docs.travis-ci.com/user/notifications/#Email-notifications

    Maybe we should start with enabling it for on_failure: change and send
    the email to php-cvs list?

    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227
  • Pierre Joye at Apr 6, 2014 at 3:10 am
    hi,
    On Sun, Apr 6, 2014 at 3:40 AM, Stas Malyshev wrote:
    Hi!
    It would be a good start to send a mail to the person introducing a CI
    build failure automatically. I think to the most part people are simply
    not aware that they introduce failures - it's not like anybody goes on
    travis half an hour after doing a commit to check whether everything
    works fine. Maybe bugging people about this is already enough to keep a
    green build most of the time?
    Travis seems to have a way to do this:
    http://docs.travis-ci.com/user/notifications/#Email-notifications

    Maybe we should start with enabling it for on_failure: change and send
    the email to php-cvs list?
    to internals, that's where discussions are supposed to happen. And if
    one fears the spam, then we he will be motivated enough to keep it all
    green :)

    Cheers,
    --
    Pierre

    @pierrejoye | http://www.libgd.org
  • Pierre Joye at Apr 6, 2014 at 3:13 am

    On Sat, Apr 5, 2014 at 8:41 PM, Stas Malyshev wrote:
    Hi!

    As many of us know, we have a setup on Travis CI which automatically
    runs unit tests for our active branches. This is very convenient for
    evaluating pull request and status of the branches and keeping us from
    making changes that break something without us noticing. However,
    there's a problem with this setup, and the problem is that CI build is
    never kept in the green for more than a a few days, the "normal" state
    of it is always failing. I think this situation is not normal and,
    frankly, embarrassing for the project, and propose to institute a policy
    to change it.
    The start would be recognizing that the goal is to keep the CI tests
    green. Once they are green, and somebody commits a change that makes
    them fail, we have the following options (note I'm not advocating for
    now any of them, just enumerating them) to fix it:

    1. Revert the change immediately, and ask the submitter to fix the pull
    (pulls can be tested too on CI too) and re-submit it when it's green.

    2. Wait for the change developer for a reasonable short time to fix the
    tests, if that does not happen, revert the change.

    3. Put the failing tests into XFAIL until they are fixed by somebody.

    4. Have somebody - e.g. RM for the branch - to modify the failing test
    to reflect the new results.

    I am not a big fan of changing tests to make it work, especially for
    stable branches. Except for warning&co additions, a test change means
    a BC break. We have to be very careful here. Indeed I refer to
    existing tests here, newly added tests to cover a bug fix is another
    story, it can fail on one platform or a specific configuration. We can
    fix them by making them configuration&system agnostic.
    These options, of course, can be used in combination and in
    case-per-case basis, but I think we need some explicit attention to the
    topic. I welcome everybody to propose suggestions about other options or
    ways we could fix this situation going forward and would like to hear
    the ideas about how we could make Travis CI tests useful and passing
    successfully, thus ensuring better quality in the project. After the
    initial discussion, I plan to write an RFC summarizing the proposals and
    have it voted on and, hopefully, accepted, so we'd have official policy
    on it, but fir now I'd also like to raise awareness of the issue and
    solicit ideas and participation of the members of the project on this topic.
    One thing I would like to consider is peer reviews for patches in
    stable branches. What you proposed goes in this direction, always go
    through PR and travis before a change can be applied.

    Cheers,
    --
    Pierre

    @pierrejoye | http://www.libgd.org
  • Stas Malyshev at Apr 6, 2014 at 3:25 am
    Hi!
    I am not a big fan of changing tests to make it work, especially for
    stable branches. Except for warning&co additions, a test change means
    I agree. But too often we face a situation where maintainer of the code
    is too busy or doesn't care enough to clean up the tests. Also, tests
    may be broken because of some deep bug that may take a lot of time to
    fix. So we need to develop a policy here. The policy may also be "don't
    care - broken tests means revert", if the community agrees with it.

    There is also the question of some tests working in one envt and not in
    another (intl tests are especially famous for it due to ICU changing
    their formats constantly) but I guess once we have a policy we can make
    tweaks and exceptions case-by-case.
    One thing I would like to consider is peer reviews for patches in
    stable branches. What you proposed goes in this direction, always go
    through PR and travis before a change can be applied.
    That's one of the purposes. If I look on the patch, I want to see nice
    green checkmark, but that is impossible if our own build is always broken.

    --
    Stanislav Malyshev, Software Architect
    SugarCRM: http://www.sugarcrm.com/
    (408)454-6900 ext. 227

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupphp-internals @
categoriesphp
postedApr 5, '14 at 6:41p
activeApr 6, '14 at 3:25a
posts6
users3
websitephp.net

People

Translate

site design / logo © 2022 Grokbase