|» Metadata||» Status|
The following dot points have been pretty much shamelessly taken from http://misko.hevery.com/code-reviewers-guide/ and other related talks, writings, etc.
I would like to propose for all new packages / PEPr proposals:
1) All new packages have unit tests (PHPUnit or phpt)
If you don't have some kind of tests in your PEPr proposal, it shouldn't go to call for votes / votes should be conditional.
They don't have to be complete, but at least a start.
For packages before being considered a stable release:
2) The unit tests should completely control the test environment
Mock out HTTP requests, database results, etc. This means really fast tests, and stops false positives.
3) Tests should cover both expected and common error cases
For example, if you are doing something over HTTP; inject a mock 404, 403 or 505 response; and expect certain exceptions to be raised.
4) Code should avoid the use of static methods which can't be mocked out.
5) Use of Agile Documentation for PHPUnit tests should be encouraged - FooTest::testShouldPerformSomeAction() instead of FooTest::testMethodName()
Hint: try phpunit --testdox YourTest some time :)
6) An AllTests.php should be provided, so that it can integrate into pear-wide test coverage.
See http://cvs.php.net/viewvc.cgi/pear/AllTests.php?view=markup and http://lauken.com/doconnor/ for a little bit more info.
7) Aim for near 100% coverage (as much coverage as is reasonably possible, at least coverage of every method)
8) All tests should run just fine from CVS/SVN.
A directory structure of:
Basically, when you
All of the require_once's should pick up CVS's first, not the installed version
9) If there is something which can't be mocked out - ie, a DB driver - and it needs configuration; the unit tests should skip, not fail.
10) Unless there's a very compelling reason, your code should work just fine with PHPUnit's backupGlobals off.
IE: Avoid using $_POST, $_GET, $_REQUEST or any other superglobals unless you are specifically designed to mess with them - HTTP_Session for instance.
11) In general, make sure you can pass dependencies in through the constructor or setters.
12) In general, your constructor should avoid doing any Real Work - it takes values and puts them into correct places.
13) In general, the new operator should not appear outside of the constructor or factory methods - otherwise this cannot be mocked out.
|» Dependencies||» Links|
|» Timeline||» Changelog|