On Mar 3, 2011, at 11:14 PM, Daniel Pittman wrote: > On Thu, Mar 3, 2011 at 21:40, Luke Kanies <[email protected]> wrote: >> On Mar 2, 2011, at 5:07 PM, Daniel Pittman wrote: >>> On Wed, Mar 2, 2011 at 16:32, Ben Hughes <[email protected]> wrote: >>>> On Wed, Mar 02, 2011 at 03:24:32PM -0800, Daniel Pittman wrote: >>>> >>>>> After every single test in the spec suite, assert that there are no >>>>> log messages at error or higher in the collected logs, before we flush >>>>> them. If there are, fail hard, and treat it as disaster. >>>> >>>> This looks a really good addition to the testing! >>>> Sure some tests will fail initially, but that's a good thing, right? >>>> >>>> Will ultimately fix more things than it breaks at the beginning. > > […] > >> I think your testing of the invariant is reasonable, but I think normal >> tests should probably use this: >> Puppet.expects(:warning) >> instead of testing that a given log is stored. That way the individual >> tests are a bit more isolated from the internals of how logs are stored. > > We are actually capturing those logs off to a special "test only" > location already, so this wouldn't be testing anything real-world, but > rather a quasi-stub mechanism for storing logs that we can treat as > API delivered by the test framework. (eg: if the logging changes, > only the test support code needs to be updated.) > > I presume with the first comment you mean rather this: > > [:warning, :err, :crit, :alert].each do |level| > Puppet.expects(:level).never end > > …since this is specifically about testing that we *don't* generate > logs at those levels which we didn't otherwise expect.
Ah; I looked for that in the code but managed not to find it. Right, that works. >> Really, you could almost say that the 'before' block should just have >> something like: >> Puppet.stubs(:warning).raises "Failed test" > > This would terminate the operating code inside puppet at that point; > now logging is fatal, and there is no mechanism to suppress that. If > it wasn't for that property I would pretty much entirely agree. > > Nick actually suggested using a smarter log target which tracked that > less destructively, which I was tempted by but couldn't see sufficient > value in. Huh; not sure why it's a bad thing to terminate the processing here, since then at least you get a stack trace and such, but I'll take your word for it that it is. >> or something similar. This way you're entirely relying on the API. You >> probably want to actually use the internal Log class, rather than Puppet >> (since classes that include the Logging module don't use Puppet.whatever, I >> think). > > We actually are using that at the moment: we have a custom "test" log > destination that captures messages to the system, and manage their > life-cycle as part of testing. So, we have zero internal changes to > achieve the current custom capture, and would still have zero if we > introduced this extra automatic assertion. Perfect; I obviously couldn't quite find how all this worked. -- Yesterday upon the stair I met a man who wasn't there. He wasn't there again today -- I think he's from the CIA. --------------------------------------------------------------------- Luke Kanies -|- http://puppetlabs.com -|- +1(615)594-8199 -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
