Re: [PEPr] +1 for Testing::Testing_DocTest
| From: | David Jean Louis | Date: | Wed, 19 Mar 2008 13:59:16 +0000 |
| Subject: | Re: [PEPr] +1 for Testing::Testing_DocTest | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-49463@lists.php.net to get a copy of this message | ||
Hey Travis, thanks for your comments !
This vote is conditional. The condition is: 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.Not sure what do you mean by "hard-coded sections" could you explain ? I am completely ok with the idea of adapting things for the future PEAR2/PHPT, I'll take a closer look at PHPT and see what I can do in order to make the two packages match.
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.Yeah, I plan to move thinks in a Tokenizer class, this will hopefully solve this problem.
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().Good idea, I'll look at this, but I want to keep the glob functionality and I may be wrong but AFAIK spl directory iterators do not provide it.
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.Well the package runs pretty well on windows (I only tested it on a vmware winXP version), as for exit codes I didn't have this problem. Thanks for the link anyway, I'll take look. David.