Refactor the $jenkins_plugins fact more, the fact will now be alphabe…

…tically sorted and more consistent

This also opens up the ability to use Puppet::Jenkins::Plugins#available inside
the plugins package provider (to be written)

lib/facter/jenkins.rb

@@ -4,32 +4,8 @@
 # jenkins plugins + versions.
 #
 #
-require 'facter'
-require 'puppet/jenkins'
-require 'puppet/jenkins/plugins'
-
-module Jenkins
-  module Facts
-    # Method to call the Facter DSL and dynamically add facts at runtime.
-    #
-    # This method is necessary to add reasonable RSpec coverage for the custom
-    # fact
-    #
-    # @return [NilClass]
-    def self.add_facts
-      Facter.add(:jenkins_plugins) do
-        confine :kernel => "Linux"
-        setcode do
-          Jenkins::Plugins.plugins
-        end
-      end
-
-      return nil
-    end
-  end
-end
-
+require 'puppet/jenkins/facts'
 
 # If we're being loaded inside the module, we'll need to go ahead and add our
 # facts then won't we?
-Jenkins::Facts.add_facts
+Puppet::Jenkins::Facts.install

lib/puppet/jenkins/facts.rb

@@ -0,0 +1,40 @@
+require 'facter'
+require 'puppet/jenkins'
+require 'puppet/jenkins/plugins'
+
+module Puppet
+  module Jenkins
+    module Facts
+      # Method to call the Facter DSL and dynamically add facts at runtime.
+      #
+      # This method is necessary to add reasonable RSpec coverage for the custom
+      # fact
+      #
+      # @return [NilClass]
+      def self.install
+        Facter.add(:jenkins_plugins) do
+          confine :kernel => "Linux"
+          setcode do
+            self.plugins_str
+          end
+        end
+        return nil
+      end
+
+      # Return a list of plugins and their versions, e.g.:
+      #   pam-auth 1.1, pmd 3.36, rake 1.7.8
+      #
+      # @return [String] Comma-separated version of "<plugin> <version>", empty
+      #   string if there are no plugins
+      def self.plugins_str
+        plugins = Puppet::Jenkins::Plugins.available
+        buffer = []
+        plugins.keys.sort.each do |plugin|
+          manifest = plugins[plugin]
+          buffer << "#{plugin} #{manifest[:plugin_version]}"
+        end
+        return buffer.join(', ')
+      end
+    end
+  end
+end

lib/puppet/jenkins/plugins.rb

@@ -36,14 +36,11 @@ def self.manifest_data(manifest_str)
         return data
       end
 
-      # Return a list of plugins and their versions, e.g.:
-      #   pam-auth 1.1, pmd 3.36, rake 1.7.8
-      #
-      # @return [String] Comma-separated version of "<plugin> <version>", empty
-      #   string if there are no plugins
-      def self.plugins
-        return '' unless exists?
-        plugins = []
+      # @return [Hash] a +Hash+ containing a mapping of a plugin name to its
+      #   manifest data
+      def self.available
+        return {} unless exists?
+        plugins = {}
         Dir.entries(directory).each do |plugin|
           # Skip useless directories
           next if (plugin == '..')
@@ -57,16 +54,14 @@ def self.plugins
           begin
             manifest = manifest_data(File.read(manifest))
             if manifest
-              version = manifest[:plugin_version]
-              plugins << "#{plugin} #{version}"
+              plugins[plugin] = manifest
             end
           rescue StandardError => ex
             # Nothing really to do about it, failing means no version which will
             # result in a new plugin if needed
           end
         end
-
-        return plugins.join(', ')
+        return plugins
       end
 
       # Determine whether or not the jenkins plugin directory exists

spec/unit/facter/plugins_spec.rb

@@ -1,36 +1,75 @@
 require 'spec_helper'
 require 'lib/facter/jenkins'
 
-describe 'jenkins_plugins fact', :type => :fact do
-  let(:fact) { Facter.fact(:jenkins_plugins) }
-  subject(:plugins) { fact.value }
+describe Puppet::Jenkins::Facts do
+  describe '.plugins_str' do
+    subject(:plugins_str) { described_class.plugins_str }
+    let(:plugins) { {} }
 
-  before :each do
-    Facter.fact(:kernel).stubs(:value).returns(kernel)
-    Jenkins::Facts.add_facts
-  end
-
-  context 'on Linux' do
-    let(:kernel) { 'Linux' }
+    before :each do
+      Puppet::Jenkins::Plugins.should_receive(:available).and_return(plugins)
+    end
 
     context 'with no plugins' do
-      it { should be_nil }
+      it { should be_instance_of String }
+      it { should be_empty }
+    end
+
+    context 'with one plugin' do
+      let(:plugins) do
+        {
+          'greenballs' => {:plugin_version => '1.1', :description => 'rspec'}
+        }
+      end
+
+      it { should be_instance_of String }
+      it { should eql 'greenballs 1.1' }
     end
 
-    context 'with plugins' do
-      let(:plugins_str) { 'ant 1.2, git 2.0.1' }
-      before :each do
-        Jenkins::Facts::Plugins.should_receive(:plugins).and_return(plugins_str)
+    context 'with multiple plugins' do
+      let(:plugins) do
+        {
+          'greenballs' => {:plugin_version => '1.1', :description => 'rspec'},
+          'git' => {:plugin_version => '1.7', :description => 'rspec'}
+        }
       end
 
-      it { should eql(plugins_str) }
+      it { should be_instance_of String }
+      it { should eql 'git 1.7, greenballs 1.1' }
     end
   end
 
-  context 'on FreeBSD' do
-    let(:kernel) { 'FreeBSD' }
+  describe 'jenkins_plugins fact', :type => :fact do
+    let(:fact) { Facter.fact(:jenkins_plugins) }
+    subject(:plugins) { fact.value }
+
+    before :each do
+      Facter.fact(:kernel).stubs(:value).returns(kernel)
+      Puppet::Jenkins::Facts.install
+    end
+
+    context 'on Linux' do
+      let(:kernel) { 'Linux' }
+
+      context 'with no plugins' do
+        it { should be_nil }
+      end
+
+      context 'with plugins' do
+        let(:plugins_str) { 'ant 1.2, git 2.0.1' }
+        before :each do
+          Jenkins::Facts::Plugins.should_receive(:plugins).and_return(plugins_str)
+        end
+
+        it { should eql(plugins_str) }
+      end
+    end
+
+    context 'on FreeBSD' do
+      let(:kernel) { 'FreeBSD' }
 
-    it { should be_nil }
+      it { should be_nil }
+    end
   end
 end
 

spec/unit/jenkins_plugins_spec.rb

@@ -33,15 +33,16 @@
     end
   end
 
-  describe '.plugins' do
-    subject(:plugins) { described_class.plugins }
+  describe '.available' do
+    subject(:available) { described_class.available }
 
     context 'when plugins do not exist' do
       before :each do
         described_class.should_receive(:exists?).and_return(false)
       end
 
-      it { should eql('') }
+      it { should be_empty }
+      it { should be_instance_of Hash }
     end
 
     context 'when plugins exist' do