On Fri, Mar 4, 2011 at 8:39 AM, Luke Kanies <[email protected]> wrote:
> On Mar 4, 2011, at 8:25 AM, Markus Roberts <[email protected]> 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. 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. -- 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.
