At first glance this doesn't look unreasonable, but the commit message could definitely use some work. Why is this a good change? What problem does it solve? I'd want to see the answers to questions like that in the commit message.
-- Please excuse my brevity; sent from my mobile device. On Jul 15, 2011, at 4:36 PM, Nan Liu <[email protected]> wrote: > Changed pluginsync so it does not load any non ruby files. > > Thanks, > > Nan > --- > lib/puppet/configurer/plugin_handler.rb | 8 ++++++-- > spec/unit/configurer/plugin_handler_spec.rb | 23 +++++++++++++++-------- > 2 files changed, 21 insertions(+), 10 deletions(-) > > diff --git a/lib/puppet/configurer/plugin_handler.rb > b/lib/puppet/configurer/plugin_handler.rb > index ae088f2..3806b58 100644 > --- a/lib/puppet/configurer/plugin_handler.rb > +++ b/lib/puppet/configurer/plugin_handler.rb > @@ -24,8 +24,12 @@ module Puppet::Configurer::PluginHandler > return if FileTest.directory?(file) > > begin > - Puppet.info "Loading downloaded plugin #{file}" > - load file > + if file =~ /.rb$/ > + Puppet.info "Loading downloaded plugin #{file}" > + load file > + else > + Puppet.info "Skipping downloaded plugin #{file}" > + end > rescue Exception => detail > Puppet.err "Could not load downloaded file #{file}: #{detail}" > end > diff --git a/spec/unit/configurer/plugin_handler_spec.rb > b/spec/unit/configurer/plugin_handler_spec.rb > index 7d99960..0fb7183 100755 > --- a/spec/unit/configurer/plugin_handler_spec.rb > +++ b/spec/unit/configurer/plugin_handler_spec.rb > @@ -75,18 +75,25 @@ describe Puppet::Configurer::PluginHandler do > @pluginhandler.download_plugins > end > > - it "should load plugins when asked to do so" do > + it "should load ruby plugins when asked to do so" do > FileTest.stubs(:exist?).returns true > - @pluginhandler.expects(:load).with("foo") > + @pluginhandler.expects(:load).with("foo.rb") > + > + @pluginhandler.load_plugin("foo.rb") > + end > + > + it "should skip non-ruby plugins when asked to do so" do > + FileTest.stubs(:exist?).returns true > + @pluginhandler.expects(:load).never > > @pluginhandler.load_plugin("foo") > end > > it "should not try to load files that don't exist" do > - FileTest.expects(:exist?).with("foo").returns false > + FileTest.expects(:exist?).with("foo.rb").returns false > @pluginhandler.expects(:load).never > > - @pluginhandler.load_plugin("foo") > + @pluginhandler.load_plugin("foo.rb") > end > > it "should not try to load directories" do > @@ -99,17 +106,17 @@ describe Puppet::Configurer::PluginHandler do > > it "should warn but not fail if loading a file raises an exception" do > FileTest.stubs(:exist?).returns true > - @pluginhandler.expects(:load).with("foo").raises "eh" > + @pluginhandler.expects(:load).with("foo.rb").raises "eh" > > Puppet.expects(:err) > - @pluginhandler.load_plugin("foo") > + @pluginhandler.load_plugin("foo.rb") > end > > it "should warn but not fail if loading a file raises a LoadError" do > FileTest.stubs(:exist?).returns true > - @pluginhandler.expects(:load).with("foo").raises LoadError.new("eh") > + @pluginhandler.expects(:load).with("foo.rb").raises LoadError.new("eh") > > Puppet.expects(:err) > - @pluginhandler.load_plugin("foo") > + @pluginhandler.load_plugin("foo.rb") > end > end > -- > 1.7.5.3 > > -- > 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.
