Thanks, I've updated my branch based on comments, and it works fine with a dummy manifest that includes a single file change, changes to multiple files and changes to a file with an alternative root. Also cherry-picked Dominic's initial test cases.
https://github.com/mikeknox/puppet/tree/feature/master/2728 But now I'm having a crash course in mocha/rspec test cases... Most of the existing tests still pass except for the "augeas execution integration" tests which now fail. I've made some updates, but it feels like the tests are too perscriptive about how the provider is working internally rather than what it's results should be. As an example, after making the testcase aware of augeas.get and augeas.match I get ... Puppet::Type::Augeas::ProviderAugeas augeas execution integration should handle remove commands Failure/Error: @resource.expects(:[]).times(2).returns(command).then.returns(context) Mocha::ExpectationError: not all expectations were satisfied unsatisfied expectations: - expected exactly twice, invoked once: #<Mock:resource>.[](any_parameters) - expected exactly once, not yet invoked: #<Mock:augeas>.save(any_parameters) - expected exactly twice, invoked once: #<Mock:augeas>.match('/augeas/events/saved') - expected exactly once, not yet invoked: #<Mock:augeas>.rm('/Jar/Jar') satisfied expectations: - allowed any number of times, not yet invoked: Signal.trap(any_parameters) - expected exactly once, invoked once: #<Mock:augeas>.close(any_parameters) - expected exactly once, invoked once: #<Mock:augeas>.get('/files/Jar/Jar') - allowed any number of times, not yet invoked: #<Puppet::Type::Augeas::ProviderAugeas:0x7f286f22d000>.get_augeas_version(any_parameters) Changes to test case: --- a/spec/unit/provider/augeas/augeas_spec.rb +++ b/spec/unit/provider/augeas/augeas_spec.rb provider.aug= augeas_stub @@ -415,10 +415,13 @@ describe provider_class do end it "should handle remove commands" do - command = "remove /Jar/Jar" + file = "/Jar/Jar" + command = "remove #{file}" context = "" @resource.expects(:[]).times(2).returns(command).then.returns(context) @augeas.expects(:rm).with("/Jar/Jar") + @augeas.expects(:match).times(2).with("/augeas/events/saved").returns(["/files#{file}"]) + @augeas.expects(:get).with("/files#{file}").returns(["/files#{file}"]) @augeas.expects(:save).returns(true) @augeas.expects(:close) @provider.execute_changes.should == :executed I'm quite concerned that I need to be digging in the "augeas execution integration" tests, but they seem to be quite sensitive to the changes I've made in the provider. Does anybody have some suggestions on this? Cheers Mike On 29 April 2011 00:13, Dominic Cleal <dcl...@redhat.com> wrote: > On 27/04/11 05:56, Mike wrote: > > I'm guessing that this has slipped through a filter somewhere, but I'm > > hoping it could be reviewed and included soon. > > Apologies Mike, I've been waiting for a chance. Now I'm at Puppet Camp, > I have free time on my hands :-) > > Comments inline: > > >> + saved_files.each do |tmp_file| > >> + saved_file = tmp_file.sub(/^\/files/, '') > >> + info(diff(saved_file, saved_file + ".augnew")) > >> + if Puppet[:noop] > >> + File.delete(saved_file + ".augnew") > >> + end > >> + end > > This diff works slightly differently to the file type, could they be > made consistent? See lib/puppet/type/file/content.rb. > > Specifically, the file type checks the show_diff setting and uses > "print" instead of "info". > > >> ensure > >> - close_augeas > >> + if Puppet[:noop] > >> + close_augeas > >> + end > >> end > > The close_augeas method should also be called when no files are changed > (return_value is false) or when in noop. > > Also when checking noop, it should check the resource noop? method which > checks the resource's noop attribute and the global setting. I'd > suggest this: > > if not return_value or resource.noop? > close_augeas > end > > Otherwise works and looks great - thanks for patching it! > > Cheers, > > -- > Dominic Cleal > Red Hat Consulting > m: +44 (0)7818 512168 > > -- > 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. > > -- 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.
