Re: [PEPr] Comment on Networking::Net_OpenSSH

Hi Daniel, thanks for your comments. Tests: I'll take a look in depth and I'll add these points on my TODO list Code: I've fixed it in the 0.1.1 version. Ciao, Luca Tests: > With the unit tests; is there any way you can mock out large chunks of it > so you don't have to setup a configuration file? > > For instance, > > > http://devel.2bopen.org/cgi-bin/trac/pear/browser/trunk/Net_OpenSSH/Net/OpenSSH.php#L268 > > proc_open creates streams for you to write to, and there's no way for the > unit tests to replace that with something else (say, a php in memory > stream) - think about ways to mock that out; and you can simulate a lot of > behaviour / control your test environment. > > > You might also be interested in PHPUnit's data provider features; which > allow you to write 1x test and supply many assorted types of input. > > Also the tests are often a good way to demonstrate how the code is used - > the way you've written them doesn't quite do that (though it does provide > coverage) > > Finally Consider naming your tests in an agile fashion > (http://www.phpunit.de/manual/3.1/en/other-uses-for-tests.html) > OpenSSH > > Code: > Might be worth swapping around some of the places where you do: > > function foo() { > if (condition) { > ... large part of function ... > > return true; > } else { > throw new Exception("BLEHHHHH!"); > } > } > > > to be: > function foo() { > if (!condition) { > throw new Exception("BLEHHHHH!"); > } > > // ... large part of function ... > return true; > } > > > Other than that, LGTM >