stas

Stas Malyshev , from Zend Technologies
(user votes do not necessarily reflect their company's views)

Contents

PHP RFC: Keeping PHPT Tests Green

Introduction

We have a setup on Travis CI which automatically runs PHPT 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 few days, the “normal” state of it is always failing. I think this situation is not normal and embarrassing for the project, and propose to institute a policy to change it.

Proposal

The start would be recognizing that the goal is to keep the CI tests green and have it officially stated. Once the tests are green, and somebody commits a change that makes them fail, we need to have an accepted policy of what to do. There are a number of ways to approach it, which may be combined and modified as needed and as common sense dictates. This RFC is meant to find out which way of handling it is the most appropriate and accepted by the comminity.

Notifications

It is proposed to have the email notifications enabled for Travis CI to inform the internals list members about the tests going from green to red.

Releases

It is proposed that the main responsibility of keeping the CI tests green will rest with the RM of the particular branch, in case of version branch. One of the responsibilities is that the release should not be made unless the CI tests are green, and the RM is primarily responsible for bringing it to green in one of the ways described below.

We will also add instructions for the RM to README.RELEASE_NOTES on how to verify the release branch and ensure everything passes.

Fixing tests

The RM also can ask anybody to act on his behalf in fixing the tests, or consent to anybody volunteering to do it. For master, any of the core developers may serve the same role. The following proposal will use “RM” as the designator of the person taking the action, with understanding from the above that this may be any core dev acting with approval of the RM on version branches, or any core dev on master unless there is a significant disagreement about how to proceed.

Then the following choices proposed for handling of bad commits (with any commit breaking CI tests presumed to be bad), which are as follows:

  1. RM will revert the change as soon as possible, and ask the submitter to fix the pull (since the pulls are tested on CI too) and re-submit it when it's green. The assumption is that pulls that are not green on CI should never be merged. Exceptions to that may be critical security fixes, and if the submitter fixes the commit within short time frame (limited by 2 days) after notifying the RM ASAP on the intention to fix.
  2. RM will wait for the change developer for a (longer) reasonable time to fix the tests, if that does not happen, RM will revert the change. Reasonable time is limited by 1 week but can be extended if circumstances warrant it.
  3. RM will put the failing tests into XFAIL until they are fixed by somebody, instead of reverting the change as per above.
  4. RM will modify the failing test to reflect the new results, after ensuring this is the intended behavior.

These policies would also apply to any future CI system we may introduce, such as Jenkins.

For the changes committed to multiple branches, the RM of the lowest branch is considered responsible, but if they are unable to resolve the problem, the RMs for other branches can step in.

Proposed PHP Version(s)

The policy would apply for all branches in active development or maintenance, starting with PHP 5.5.

References

Versions

Version Changed Date
1.0 Created the RFC

Votes

An option needs 50%+1 votes to win

Accept the description in this RFC as an official policy of the PHP project with regard to the tests (100% approved)
User Vote
ab Yes
ajf Yes
datibbaw Yes
derick Yes
indeyets Yes
jpauli Yes
krakjoe Yes
lstrojny Yes
malukenho Yes
mbeccati Yes
mike Yes
pajoye Yes
sebastian Yes
stas Yes
tyrael Yes
yohgaki Yes
Choose one or more of the four options above of handling bad commits (100% approved)
User Vote
ab Update test
ajf Update test
datibbaw Update test
derick Update test
indeyets Revert ASAP
krakjoe Update test
levim Update test
lstrojny Revert ASAP
malukenho XFAIL
mbeccati Revert ASAP
mike Update test
pajoye Revert ASAP
philip Update test
stas Update test
tyrael XFAIL
yohgaki Update test