Vote Details for "Testing_DocTest" by tswicegood
| » Details |
|---|
|
| » Comment |
|
This looks good, but prior to a 1.0 I would want to see all of the hard-coded sections removed in favor of extensible sections so the PEAR DocTest and the PEAR2/PHPT_DocTest will match. Getting rid of the specific set of section names will be required for this to work. The number of privates is a bit high: 12 in the Parser alone. One or two is generally ok, but if you have a large # of private methods, you generally need to refactor to separate concerns better (i.e., if it shouldn't be part of the public interface, it shouldn't be handled by that object). It leads to better re-usability, along with more testable code. As it stands, there's large chunks of this code that are not directly testable. On a stylistic point (these aren't part of the conditional vote, just comments), I'd rename Config::singleton() Config::getInstance() so as to describe what is happening better. I'd also use the SPL directory iterators and filters to handle finding files in place of glob(). The various proc_* calls will have a hard time functioning properly on Windows. I've also had issue with proc_close() returning the proper exit code and had to rely on piping it out to a 4th stream in order to get a valid value on every run. See PHPT's CodeRunner_Driver classes for examples of what I had to do with proc_open() and how to execute against WScriptShell using the COM object on Windows. The latter is still a bit rough around the edges, but will get you started. |