Keeping PHPT Tests Green
- Currently implemented
- Target version: 5.5
- Approved at 100%
PHP RFC: Keeping PHPT Tests Green
- Version: 1.0
- Date: 2014-04-13
- Author: Stas Malyshev (firstname.lastname@example.org)
- Status: Accepted
- First Published at: http://wiki.php.net/rfc/travis_ci
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.
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.
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.
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.
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:
- 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.
- 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.
- RM will put the failing tests into XFAIL until they are fixed by somebody, instead of reverting the change as per above.
- 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.
|1.0||Created the RFC|
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)|
|Choose one or more of the four options above of handling bad commits (100% approved)|