On Mar 4, 2011, at 2:19 PM, Paul Berry wrote: > On Fri, Mar 4, 2011 at 8:39 AM, Luke Kanies <l...@puppetlabs.com> wrote: > On Mar 4, 2011, at 8:25 AM, Markus Roberts <mar...@puppetlabs.com> wrote: > >> > > 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. >> >> 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. >> >> Because sometimes we want to write a test that asserts that an error will be >> logged under certain circumstances, or that some condition is or isn't >> modified when a condition which results in a logged error occurs. Such >> tests would always either fail or pass for the wrong reasons. > > I thought processing would only stop if the log wasn't stubbed, in which case > you've got a failure you didn't plan on. If you correctly stub/expect the > logs, then processing should absolutely continue. > > Am I missing something? > > I think the misunderstanding here is coming from some ambiguous statements > that we're interpreting differently. > > Luke, when you suggested having a 'before' block that said > 'Puppet.stubs(:warning).raises "Failed test"' (or something similar), did you: > > (1) accidentally say :warning when you actually meant :err? > (2) mean only to talk only about warnings? > (3) mean to talk about all four of :warning, :err, :crit, and :alert, and use > :warning to stand for all four? > > If it's (1), then I can see where you are coming from (however, see comments > below). If it's (2), then I think this wouldn't address the issue Daniel > initially set out to solve (tests passing when they actually should fail > because an exception is converted to a call to err()). If it's (3), then I > think this might cause a lot of false positives, because there may be a lot > of non-problematic tests out there that cause :crit, :alert, and :warning log > messages.
I meant #3, because I thought the point here was to test that we knew when this kind of logging happened. But I probably misinterpreted somewhere, and it's more about catching errors than logs. Or something. And unrelated, but I'm not sure we generate a single crit or alert log anywhere in the system. At least, I don't know of any. > Secondly, when you suggested having a 'before' block that said > 'Puppet.stubs(:warning).raises "Failed test"', were you intending this as: > > (1) Individual 'before' blocks that stub out this method for some tests and > not others? > (2) A global 'before' block that stubs out this method (or its equivalent) > for all tests? > > If it's (1), then this suggestion has a significant disadvantage compared to > Daniel's proposal, which is that we would have to remember to include the > 'before' block in all the places where there is any danger of an exception > getting caught and translated into a call to err(). Daniel's proposal has > the advantage that we don't have to remember anything; _all_ of our tests > will be protected. If it's (2), then this suggestion has a significant > disadvantage that in the tests where we want to verify that Puppet.warning() > functions properly we won't be able to, because it is already stubbed out. #2. I agree, there is that downside. Is it possible to get a stack trace that leads us to where the log is being generated in the current proposal? -- A cult is a religion with no political power. -- Tom Wolfe --------------------------------------------------------------------- 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 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.
