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.
> 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 <[email protected]>
✆ 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 [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.
