Merge branch 'bug/2.7.x/7132-a-summary-with-a-newline-is-accepted' into 2.7.x

5 files changed, 100 insertions, 39 deletions

diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb
index 5e9355061..888e4ecad 100644
--- a/lib/puppet/interface.rb
+++ b/lib/puppet/interface.rb
@@ -68,12 +68,31 @@ class Puppet::Interface
     self.default_format = format.to_sym
   end
 
-  attr_accessor :summary
+  ########################################################################
+  # Documentation.  We currently have to rewrite both getters because we share
+  # the same instance between build-time and the runtime instance.  When that
+  # splits out this should merge into a module that both the action and face
+  # include. --daniel 2011-04-17
+  attr_accessor :summary, :description
   def summary(value = nil)
-    @summary = value unless value.nil?
+    self.summary = value unless value.nil?
     @summary
   end
+  def summary=(value)
+    value = value.to_s
+    value =~ /\n/ and
+      raise ArgumentError, "Face summary should be a single line; put the long text in 'description' instead."
+
+    @summary = value
+  end
+
+  def description(value = nil)
+    self.description = value unless value.nil?
+    @description
+  end
+
 
+  ########################################################################
   attr_reader :name, :version
 
   def initialize(name, version, &block)
@@ -90,26 +109,11 @@ class Puppet::Interface
 
   # Try to find actions defined in other files.
   def load_actions
-    path = "puppet/face/#{name}"
-
-    loaded = []
-    [path, "#{name}@#{version}/#{path}"].each do |path|
-      Puppet::Interface.autoloader.search_directories.each do |dir|
-        fdir = ::File.join(dir, path)
-        next unless FileTest.directory?(fdir)
-
-        Dir.chdir(fdir) do
-          Dir.glob("*.rb").each do |file|
-            aname = file.sub(/\.rb/, '')
-            if loaded.include?(aname)
-              Puppet.debug "Not loading duplicate action '#{aname}' for '#{name}' from '#{fdir}/#{file}'"
-              next
-            end
-            loaded << aname
-            Puppet.debug "Loading action '#{aname}' for '#{name}' from '#{fdir}/#{file}'"
-            require "#{Dir.pwd}/#{aname}"
-          end
-        end
+    Puppet::Interface.autoloader.search_directories.each do |dir|
+      Dir.glob(File.join(dir, "puppet/face/#{name}", "*.rb")).each do |file|
+        action = file.sub(dir, '').sub(/^[\\\/]/, '').sub(/\.rb/, '')
+        Puppet.debug "Loading action '#{action}' for '#{name}' from '#{dir}/#{action}.rb'"
+        require(action)
       end
     end
   end
diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 412e39449..f6273d1c2 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -24,8 +24,21 @@ class Puppet::Interface::Action
   attr_reader :name
   def to_s() "#{@face}##{@name}" end
 
-  attr_accessor :default, :summary
+  attr_accessor :default
 
+  ########################################################################
+  # Documentation stuff, whee!
+  attr_accessor :summary, :description
+  def summary=(value)
+    value = value.to_s
+    value =~ /\n/ and
+      raise ArgumentError, "Face summary should be a single line; put the long text in 'description' instead."
+
+    @summary = value
+  end
+
+
+  ########################################################################
   # Initially, this was defined to allow the @action.invoke pattern, which is
   # a very natural way to invoke behaviour given our introspection
   # capabilities.   Heck, our initial plan was to have the faces delegate to
diff --git a/spec/shared_behaviours/documentation_on_faces.rb b/spec/shared_behaviours/documentation_on_faces.rb
new file mode 100644
index 000000000..effca678c
--- /dev/null
+++ b/spec/shared_behaviours/documentation_on_faces.rb
@@ -0,0 +1,35 @@
+# encoding: UTF-8
+shared_examples_for "documentation on faces" do
+  context "description" do
+    describe "#summary" do
+      it "should accept a summary" do
+        text = "this is my summary"
+        expect { subject.summary = text }.not_to raise_error
+        subject.summary.should == text
+      end
+
+      it "should accept a long, long, long summary" do
+        text = "I never know when to stop with the word banana" + ("na" * 1000)
+        expect { subject.summary = text }.not_to raise_error
+        subject.summary.should == text
+      end
+
+      it "should reject a summary with a newline" do
+        expect { subject.summary = "with\nnewlines" }.
+          to raise_error ArgumentError, /summary should be a single line/
+      end
+    end
+
+    describe "#description" do
+      it "should accept a description" do
+        subject.description = "hello"
+        subject.description.should == "hello"
+      end
+
+      it "should accept a description with a newline" do
+        subject.description = "hello \n my \n fine \n friend"
+        subject.description.should == "hello \n my \n fine \n friend"
+      end
+    end
+  end
+end
diff --git a/spec/unit/interface/action_spec.rb b/spec/unit/interface/action_spec.rb
index fe2409b35..5058ffc7f 100755
--- a/spec/unit/interface/action_spec.rb
+++ b/spec/unit/interface/action_spec.rb
@@ -371,4 +371,13 @@ describe Puppet::Interface::Action do
       end
     end
   end
+
+  it_should_behave_like "documentation on faces" do
+    subject do
+      face = Puppet::Interface.new(:action_documentation, '0.0.1') do
+        action :documentation do end
+      end
+      face.get_action(:documentation)
+    end
+  end
 end
diff --git a/spec/unit/interface_spec.rb b/spec/unit/interface_spec.rb
index e52b45d8a..50ae9c711 100755
--- a/spec/unit/interface_spec.rb
+++ b/spec/unit/interface_spec.rb
@@ -33,7 +33,7 @@ describe Puppet::Interface do
 
   describe "#define" do
     it "should register the face" do
-      face = subject.define(:face_test_register, '0.0.1')
+      face  = subject.define(:face_test_register, '0.0.1')
       face.should == subject[:face_test_register, '0.0.1']
     end
 
@@ -51,22 +51,16 @@ describe Puppet::Interface do
       subject.new(:foo, '1.0.0').should respond_to(:summary=).with(1).arguments
     end
 
-    it "should set the summary text" do
-      text = "hello, freddy, my little pal"
-      subject.define(:face_test_summary, '1.0.0') do
-        summary text
-      end
-      subject[:face_test_summary, '1.0.0'].summary.should == text
-    end
-
-    it "should support mutating the summary" do
-      text = "hello, freddy, my little pal"
-      subject.define(:face_test_summary, '1.0.0') do
-        summary text
+    # Required documentation methods...
+    { :summary     => "summary",
+      :description => "This is the description of the stuff\n\nWhee"
+    }.each do |attr, value|
+      it "should support #{attr} in the builder" do
+        face = subject.new(:builder, '1.0.0') do
+          self.send(attr, value)
+        end
+        face.send(attr).should == value
       end
-      subject[:face_test_summary, '1.0.0'].summary.should == text
-      subject[:face_test_summary, '1.0.0'].summary = text + text
-      subject[:face_test_summary, '1.0.0'].summary.should == text + text
     end
   end
 
@@ -204,4 +198,10 @@ describe Puppet::Interface do
       end
     end
   end
+
+  it_should_behave_like "documentation on faces" do
+    subject do
+      Puppet::Interface.new(:face_documentation, '0.0.1')
+    end
+  end
 end