On Fri, Aug 28, 2009 at 2:22 AM, Eric Wilhelm<enoba...@gmail.com> wrote: > # from David Golden > # on Thursday 27 August 2009 19:08: > >>> I think this END block is redundant given a temp dir with CLEANUP => >>> 1. >> >>remove() does a chdir out of the directory. That's important or the >>tempdir can't be removed on Windows. At least, that's my best take on >>the situation. > > Ah. That might be something we lost in r12453 by changing to > File::Temp, but it looks like we still have an END block in MBTest.pm. > Either way, the abstraction needs to be good enough that the caller > doesn't have to remember to do the right thing.
I agree. I'll admit that some of I'm doing is still cargo-cult, but I've tried to improve the abstractions a bit as I work with them, which I haven't really done "from scratch" until recently. The README was mostly an attempt to capture my thought process and to make it easier for the next person. Looking at MBTest, it has an END block to chdir so the END in my example isn't really necessary. But it's a code smell to me to rely on such action at a distance to do the right thing. A couple things other things I still find a bit clunky: * capturing output when testing the command line API. (There's surprisingly little command line testing, actually.) * regen/cleanup -- I'm not entirely sure when I should be cleaning things up. When I change the distribution, I feel like I should be running "Build realclean" or an equivalent and then always rerunning Build.PL. Anything that repetitive should probably be automated away. * @INC manipulations in MBTest BEGIN blocks. At first I thought it was duplicating the @INC manipulation for "t/lib" at the top of *.t files, then I realized it was doing the equivalent for "t/bundled" but in a more complicated way. Is it more correct? Should we be doing it for "t/lib" too? Or should we just add t/bundled at the same time we do t/lib? Plus the whole thing messes with @INC and $^X, but only for perl core testing, which given comments doesn't seem right. * ensure_blib doesn't sound like a test function even though it is. That's a nit, but could confuse someone trying to count tests. That could be fixed with a big find/replace commit, but I just haven't gotten around to it. * general lack of docs for MBTest. The POD for DistGen is nice to have. -- David
