David Jean Louis [2008-12-29 13:37 UTC] Hi Dave,
Very good initiative.
All tests pass and phpcs is silent on the the "Image" directory.
Just a minor note:
* The test files contain tabs instead of spaces (probably inherited from the old code)... you should "s/\t/ /" them;
* You should remove the closing php tag (?>), it's not needed and as it's an image package it will prevent breaking image generation if there's extra space after the closing tag.
--
David
Daniel O'Connor [2009-01-15 13:11 UTC] 1)
parent::setUp() isn't needed if you jsut extend PHPUnit_Framework_TestCase:
protected function setUp()
{
parent::setUp();
$this->_Image_Barcode2 = new Image_Barcode2();
}
2) Image_Barcode2Test doesn't appear to have assertations in it - have I missed something?
3) Consider 'agile documentation' for Image_Barcode2Test and others
http://www.phpunit.de/manual/3.1/en/other-uses-for-tests.html
or run current tests with
phpunit --testdocs
4) Though it breaks expectations from the previous package, I'd kind of like to see only returning things from Image_Barcode2::draw(); and possibly a 'render helper' type method which you can call to help with output to the browser.
IE:
Image_Barcode2::displayToBrowser($image->draw());
More testable that way.
5) Test cases don't cover off execution paths which raise exceptions - that would be nice to have going forward
6) I don't know if you want to go down this path; but pre-rendering a number of images and creating test cases to ensure they render the same way might be worth thinking about - will help you catch subtle breakages; but also could record a lot of false positives (two systems use slightly different image encoding libraries => false failure of test)
Otherwise, pretty neat and happy to have you on board.
Daniel O'Connor [2009-01-15 13:12 UTC] oops, phpunit --testdox even
Michael Gauthier [2011-01-30 22:00 UTC] Dave,
The latest comments on this proposal indicate it is looking good. Are you going to continue the proposal and get it into PEAR?
Dave Hall [2011-02-14 14:35 UTC] I hope to find some time to make this happen in the coming weeks. I finished working on the project which relied on this. I need to spend some time polishing Image_Diff before I can get this accepted.
|
