Skip to content

Commit

Permalink
tune syntax error message output, handle invalid plugin names that ar…
Browse files Browse the repository at this point in the history
…en't syntax errors
  • Loading branch information
Claire McQuin committed Dec 19, 2013
1 parent 70db970 commit 1caba37
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 0 deletions.
6 changes: 6 additions & 0 deletions lib/ohai/dsl/plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ module Ohai

# For plugin namespacing
module NamedPlugin
def self.valid_name?(name)
name.is_a?(Symbol) && name.to_s.match(/^[^A-Z]|_/).nil?
end

# dealing with ruby 1.8
if Module.method(:const_defined?).arity == 1
def self.strict_const_defined?(const)
Expand All @@ -40,6 +44,8 @@ def self.strict_const_defined?(const)
end

def self.plugin(name, &block)
raise Ohai::Exceptions::InvalidPluginName, "#{name} is not a valid plugin name. A valid plugin name is a symbol which begins with a capital letter and contains no underscores" unless NamedPlugin.valid_name?(name)

plugin = nil

if NamedPlugin.strict_const_defined?(name)
Expand Down
1 change: 1 addition & 0 deletions lib/ohai/exception.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
module Ohai
module Exceptions
class Exec < RuntimeError; end
class InvalidPluginName < Exception; end
class IllegalPluginDefinition < Exception; end
class AttributeNotFound < Exception; end
class ProviderNotFound < Exception; end
Expand Down
2 changes: 2 additions & 0 deletions lib/ohai/loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ def load_v7_plugin(contents, plugin_path)
plugin = klass.new(@controller.data) unless klass.nil?
rescue SystemExit, Interrupt
raise
rescue Ohai::Exceptions::InvalidPluginName => e
Ohai::Log.warn("Invalid name for plugin at #{plugin_path}: #{e.message}")
rescue Ohai::Exceptions::IllegalPluginDefinition => e
Ohai::Log.warn("Plugin at #{plugin_path} is not properly defined: #{e.inspect}")
rescue NoMethodError => e
Expand Down
20 changes: 20 additions & 0 deletions spec/unit/dsl/plugin_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,26 @@
@name = :Test
end

describe "when the plugin is named incorrectly" do
context "because the plugin name doesn't start with a capital letter" do
it "should raise an Ohai::Exceptions::InvalidPluginName exception" do
expect{ Ohai.plugin(:badName) { } }.to raise_error(Ohai::Exceptions::InvalidPluginName, /badName is not a valid plugin name/)
end
end

context "because the plugin name contains an underscore" do
it "should raise an Ohai::Exceptions::InvalidPluginName exception" do
expect{ Ohai.plugin(:Bad_Name) { } }.to raise_error(Ohai::Exceptions::InvalidPluginName, /Bad_Name is not a valid plugin name/)
end
end

context "because the plugin name isn't a symbol" do
it "should raise an Ohai::Exceptions::InvalidPluginName exception" do
expect{ Ohai.plugin(1138) { } }.to raise_error(Ohai::Exceptions::InvalidPluginName, /1138 is not a valid plugin name/)
end
end
end

describe "#version" do
it "should save the plugin version as :version7" do
plugin = Ohai.plugin(@name) { }
Expand Down
24 changes: 24 additions & 0 deletions spec/unit/loader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,19 @@
Ohai.plugin(:1nval!d) do
provides "not_a_symbol"
end
EOF

with_plugin("no_end.rb", <<EOF)
Ohai.plugin(:NoEnd) do
provides "fish_oil"
collect_data do
end
EOF

with_plugin("bad_name.rb", <<EOF)
Ohai.plugin(:you_give_plugins_a_bad_name) do
provides "that/one/song"
end
EOF

describe "load_plugin() method" do
Expand Down Expand Up @@ -168,6 +181,17 @@
expect{ @loader.load_plugin(path_to("no_end.rb")) }.not_to raise_error
end
end

describe "when the plugin has an invalid name" do
it "should log an invalid plugin name warning" do
Ohai::Log.should_receive(:warn).with(/Invalid name for plugin/)
@loader.load_plugin(path_to("bad_name.rb"))
end

it "should not raise an error" do
expect{ @loader.load_plugin(path_to("bad_name.rb")) }.not_to raise_error
end
end
end
end
end

0 comments on commit 1caba37

Please sign in to comment.