On Thu, Mar 3, 2011 at 21:40, Luke Kanies <l...@puppetlabs.com> wrote: > On Mar 2, 2011, at 5:07 PM, Daniel Pittman wrote: >> On Wed, Mar 2, 2011 at 16:32, Ben Hughes <b...@puppetlabs.com> 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. > 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. > 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. Daniel -- ⎋ Puppet Labs Developer – http://puppetlabs.com ✉ Daniel Pittman <dan...@puppetlabs.com> ✆ Contact me via gtalk, email, or phone: +1 (877) 575-9775 ♲ Made with 100 percent post-consumer electrons -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to puppet-dev@googlegroups.com. To unsubscribe from this group, send email to puppet-dev+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.