On Wed, Sep 29, 2010 at 10:11 AM, Paul Berry <[email protected]> wrote: > Whoops, looks like the spec tests now fail when not running on Mac OSX. > Since the launchd provider is OSX-specific, I've added a confine to prevent > the tests from running on other OSes.
Is this what we should be doing? I was under the impression we were meant to abstract away platform dependent stuff in our specs? Or is it the case that doing this too much has actually ended up hiding real issues? The specs I've written for OS X would have been much simpler if I knew they'd only run on OS X :) > Squashed this diff into the commit > (http://github.com/stereotype441/puppet/tree/ticket/2.6.x/4025): > > diff --git a/spec/unit/provider/service/launchd_spec.rb > b/spec/unit/provider/service/launchd_spec.rb > index 116ba21..05d7697 100755 > --- a/spec/unit/provider/service/launchd_spec.rb > +++ b/spec/unit/provider/service/launchd_spec.rb > @@ -95,6 +95,7 @@ describe provider_class do > end > > describe "when checking whether the service is enabled on OS X 10.6" do > + confine "Not running on OSX" => (Facter.value(:operatingsystem) == > "Darwin") > it "should return true if the job plist says disabled is true and the > global overrides says disabled is false" do > provider_class.stubs(:get_macosx_version_major).returns("10.6") > @provider.stubs(:plist_from_label).returns(["foo", {"Disabled" => > true}]) > > On Fri, Sep 24, 2010 at 2:52 PM, Paul Berry <[email protected]> wrote: >> >> Modified the launchd provider to use OSX's "plutil" command to read >> plists. This allows it to handle properly lists in both XML format >> (which was the default before OSX 10.4) and binary format (which is >> the default starting with OSX 10.4). >> >> Launchd continues to write out propertly lists in XML format. This is >> not a problem because the operating system is able to understand both >> formats. >> >> Signed-off-by: Paul Berry <[email protected]> >> --- >> lib/puppet/provider/service/launchd.rb | 25 >> ++++++++++++++++--------- >> spec/unit/provider/service/launchd_spec.rb | 10 +++++----- >> 2 files changed, 21 insertions(+), 14 deletions(-) >> >> diff --git a/lib/puppet/provider/service/launchd.rb >> b/lib/puppet/provider/service/launchd.rb >> index 9703595..b296e0a 100644 >> --- a/lib/puppet/provider/service/launchd.rb >> +++ b/lib/puppet/provider/service/launchd.rb >> @@ -38,6 +38,7 @@ Puppet::Type.type(:service).provide :launchd, :parent => >> :base do >> >> commands :launchctl => "/bin/launchctl" >> commands :sw_vers => "/usr/bin/sw_vers" >> + commands :plutil => "/usr/bin/plutil" >> >> defaultfor :operatingsystem => :darwin >> confine :operatingsystem => :darwin >> @@ -52,6 +53,12 @@ Puppet::Type.type(:service).provide :launchd, :parent >> => :base do >> Launchd_Overrides = >> "/var/db/launchd.db/com.apple.launchd/overrides.plist" >> >> >> + # Read a plist, whether its format is XML or in Apple's "binary1" >> + # format. >> + def self.read_plist(path) >> + Plist::parse_xml(plutil('-convert', 'xml1', '-o', '-', path)) >> + end >> + >> # returns a label => path map for either all jobs, or just a single >> # job if the label is specified >> def self.jobsearch(label=nil) >> @@ -62,8 +69,7 @@ Puppet::Type.type(:service).provide :launchd, :parent => >> :base do >> next if f =~ /^\..*$/ >> next if FileTest.directory?(f) >> fullpath = File.join(path, f) >> - job = Plist::parse_xml(fullpath) >> - if job and job.has_key?("Label") >> + if FileTest.file?(fullpath) and job = read_plist(fullpath) and >> job.has_key?("Label") >> if job["Label"] == label >> return { label => fullpath } >> else >> @@ -118,8 +124,11 @@ Puppet::Type.type(:service).provide :launchd, :parent >> => :base do >> def plist_from_label(label) >> job = self.class.jobsearch(label) >> job_path = job[label] >> - job_plist = Plist::parse_xml(job_path) >> - raise Puppet::Error.new("Unable to parse launchd plist at path: >> #{job_path}") if not job_plist >> + if FileTest.file?(job_path) >> + job_plist = self.class.read_plist(job_path) >> + else >> + raise Puppet::Error.new("Unable to parse launchd plist at path: >> #{job_path}") >> + end >> [job_path, job_plist] >> end >> >> @@ -200,9 +209,7 @@ Puppet::Type.type(:service).provide :launchd, :parent >> => :base do >> job_plist_disabled = job_plist["Disabled"] if >> job_plist.has_key?("Disabled") >> >> if self.class.get_macosx_version_major == "10.6": >> - overrides = Plist::parse_xml(Launchd_Overrides) >> - >> - unless overrides.nil? >> + if FileTest.file?(Launchd_Overrides) and overrides = >> self.class.read_plist(Launchd_Overrides) >> if overrides.has_key?(resource[:name]) >> overrides_disabled = overrides[resource[:name]]["Disabled"] if >> overrides[resource[:name]].has_key?("Disabled") >> end >> @@ -227,7 +234,7 @@ Puppet::Type.type(:service).provide :launchd, :parent >> => :base do >> # versions this is stored in the job plist itself. >> def enable >> if self.class.get_macosx_version_major == "10.6" >> - overrides = Plist::parse_xml(Launchd_Overrides) >> + overrides = self.class.read_plist(Launchd_Overrides) >> overrides[resource[:name]] = { "Disabled" => false } >> Plist::Emit.save_plist(overrides, Launchd_Overrides) >> else >> @@ -242,7 +249,7 @@ Puppet::Type.type(:service).provide :launchd, :parent >> => :base do >> >> def disable >> if self.class.get_macosx_version_major == "10.6" >> - overrides = Plist::parse_xml(Launchd_Overrides) >> + overrides = self.class.read_plist(Launchd_Overrides) >> overrides[resource[:name]] = { "Disabled" => true } >> Plist::Emit.save_plist(overrides, Launchd_Overrides) >> else >> diff --git a/spec/unit/provider/service/launchd_spec.rb >> b/spec/unit/provider/service/launchd_spec.rb >> index 320ee3a..116ba21 100755 >> --- a/spec/unit/provider/service/launchd_spec.rb >> +++ b/spec/unit/provider/service/launchd_spec.rb >> @@ -98,19 +98,19 @@ describe provider_class do >> it "should return true if the job plist says disabled is true and the >> global overrides says disabled is false" do >> provider_class.stubs(:get_macosx_version_major).returns("10.6") >> @provider.stubs(:plist_from_label).returns(["foo", {"Disabled" => >> true}]) >> - Plist.stubs(:parse_xml).returns({...@resource[:name] => {"Disabled" => >> false}}) >> + �[email protected](:read_plist).returns({...@resource[:name] => >> {"Disabled" => false}}) >> @provider.enabled?.should == :true >> end >> it "should return false if the job plist says disabled is false and >> the global overrides says disabled is true" do >> provider_class.stubs(:get_macosx_version_major).returns("10.6") >> @provider.stubs(:plist_from_label).returns(["foo", {"Disabled" => >> false}]) >> - Plist.stubs(:parse_xml).returns({...@resource[:name] => {"Disabled" => >> true}}) >> + �[email protected](:read_plist).returns({...@resource[:name] => >> {"Disabled" => true}}) >> @provider.enabled?.should == :false >> end >> it "should return true if the job plist and the global overrides have >> no disabled keys" do >> provider_class.stubs(:get_macosx_version_major).returns("10.6") >> @provider.stubs(:plist_from_label).returns(["foo", {}]) >> - Plist.stubs(:parse_xml).returns({}) >> + �[email protected](:read_plist).returns({}) >> @provider.enabled?.should == :true >> end >> end >> @@ -182,7 +182,7 @@ describe provider_class do >> describe "when enabling the service on OS X 10.6" do >> it "should write to the global launchd overrides file once" do >> provider_class.stubs(:get_macosx_version_major).returns("10.6") >> - Plist.stubs(:parse_xml).returns({}) >> + �[email protected](:read_plist).returns({}) >> Plist::Emit.expects(:save_plist).once >> @provider.enable >> end >> @@ -191,7 +191,7 @@ describe provider_class do >> describe "when disabling the service on OS X 10.6" do >> it "should write to the global launchd overrides file once" do >> provider_class.stubs(:get_macosx_version_major).returns("10.6") >> - Plist.stubs(:parse_xml).returns({}) >> + �[email protected](:read_plist).returns({}) >> Plist::Emit.expects(:save_plist).once >> @provider.enable >> end >> -- >> 1.7.2 >> >> -- >> 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. >> > > -- > 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. > -- nigel -- 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.
