On Fri, Aug 15, 2008 at 05:13:59PM -0700, Danek Duvall wrote: > On Wed, Jul 30, 2008 at 05:19:28PM -0700, Brad Hall wrote: > > > Link to CR: http://cr.opensolaris.org/~bhall/bug-2690/ > > Damned shame that class decorators aren't available until Python 2.6 -- > this would be the perfect use for them. > > Alternatively, you could use function decorators to decorate individual > tests that required a fresh depot. That could be extended at some point in > the future to have a decorator for tests that required (or didn't require) > a fresh image, too. > > Or maybe all you need is a method that a test can call to re-initialize all > its depots? > > Perhaps you considered these designs, I dunno. But if you did, please > explain why you decided against them.
So basically this was the most generic/simple way I could think of at the time, as it essentially simulates a singleton which would be close to what we'd end up with if we used class decorators. I like the idea about having a decorator to provide a clean depot for a test function, and think that would be useful, since that gets rid of the test-ordering issue I've introduced with this change. I'll play around with that and see what comes out of it. But as we talked about, this gets us most of the way to where we want to be at this point in time. > > I added a variable to the classes to denote whether this should be > > enabled for a given class or not named 'static'. I couldn't think of a > > better name at the time so I'm open to suggestions if people think that's > > a bad name for this :) > > I'll throw out "reuse_depot". Just reading through the webrev, "static" is > already getting really confusing. OK, I had already changed to persistent_depot in the coming webrev -- I like your reuse-depot better but it makes the language in comments sound funny so I'll stick with persistent for now. > testutils.py: > > - line 552-558: I don't like leaving in commented-out code like this. > Either yank it entirely, or enhance the else to check to see if the > test is not "static". OK, gone. > pkg5unittest.py: > > - line 49: Please refrain from using single-line if statements like this Fixed > - line 52-54: combine as > > static = getattr(self, "static", False) Didn't realize you could do that :) Fixed. > - line 57: Well, we do need to call it for the first one. You might add > to the comment saying that's done down in the testsuite class below. > And capitalize setUp() properly (on 228, too). Added comment, fixed capitalization. > - line 83: space before colon Fixed > - line 216-218: use extended getattr() call as above. Done > - line 220: no need for parens around lvalue (fixed) > - line 221, 236: do these need to be in try blocks as they are in > Pkg5TestCase.run()? Good catch, fixed. > Thanks, > Danek New webrev: http://cr.opensolaris.org/~bhall/bug-2690-3/ Thanks, Brad _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
