(puppetlabsGH-141) Add Puppet Strings Helper tests

Previously a Puppet Strings Helper was added however there no tests for the
metadata extraction.  This commit:

* Adds tests to ensure that YARD/Puppet Strings extracts the metadata correctly
* Converts the Strings Helper module into a class to make it easier to test
* Updates the Puppet 4 function fixture to be a more complete example

lib/puppet-languageserver-sidecar/puppet_strings_helper.rb

@@ -2,50 +2,12 @@
 
 module PuppetLanguageServerSidecar
   module PuppetStringsHelper
-    # Returns a FileDocumentation object for a given path
-    #
-    # @param [String] path The absolute path to the file that will be documented
-    # @param [PuppetLanguageServerSidecar::Cache] cache A Sidecar cache which stores already parsed documents as serialised FileDocumentation objects
-    # @return [FileDocumentation, nil] Returns the documentation for the path, or nil if it cannot be extracted
-    def self.file_documentation(path, cache = nil)
-      return nil unless require_puppet_strings
-      @helper_cache = FileDocumentationCache.new if @helper_cache.nil?
-      return @helper_cache.document(path) if @helper_cache.path_exists?(path)
-
-      # Load from the permanent cache
-      @helper_cache.populate_from_sidecar_cache!(path, cache) unless cache.nil? || !cache.active?
-      return @helper_cache.document(path) if @helper_cache.path_exists?(path)
-
-      PuppetLanguageServerSidecar.log_message(:debug, "[PuppetStringsHelper::file_documentation] Fetching documentation for #{path}")
-
-      setup_yard!
-
-      # For now, assume a single file path
-      search_patterns = [path]
-
-      # Format the arguments to YARD
-      args = ['doc']
-      args << '--no-output'
-      args << '--quiet'
-      args << '--no-stats'
-      args << '--no-progress'
-      args << '--no-save'
-      args << '--api public'
-      args << '--api private'
-      args << '--no-api'
-      args += search_patterns
-
-      # Run YARD
-      ::YARD::CLI::Yardoc.run(*args)
-
-      # Populate the documentation cache from the YARD information
-      @helper_cache.populate_from_yard_registry!
-
-      # Save to the permanent cache
-      @helper_cache.save_to_sidecar_cache(path, cache) unless cache.nil? || !cache.active?
+    def self.instance
+      @instance ||= Helper.new
+    end
 
-      # Return the documentation details
-      @helper_cache.document(path)
+    def self.file_documentation(path, cache = nil)
+      instance.file_documentation(path, cache)
     end
 
     def self.require_puppet_strings
@@ -63,15 +25,61 @@ def self.require_puppet_strings
       end
       @puppet_strings_loaded
     end
-    private_class_method :require_puppet_strings
 
     def self.setup_yard!
       unless @yard_setup # rubocop:disable Style/GuardClause
         ::PuppetStrings::Yard.setup!
         @yard_setup = true
       end
     end
-    private_class_method :setup_yard!
+
+    class Helper
+      # Returns a FileDocumentation object for a given path
+      #
+      # @param [String] path The absolute path to the file that will be documented
+      # @param [PuppetLanguageServerSidecar::Cache] cache A Sidecar cache which stores already parsed documents as serialised FileDocumentation objects
+      # @return [FileDocumentation, nil] Returns the documentation for the path, or nil if it cannot be extracted
+      def file_documentation(path, cache = nil)
+        return nil unless PuppetLanguageServerSidecar::PuppetStringsHelper.require_puppet_strings
+        @helper_cache = FileDocumentationCache.new if @helper_cache.nil?
+        return @helper_cache.document(path) if @helper_cache.path_exists?(path)
+
+        # Load from the permanent cache
+        @helper_cache.populate_from_sidecar_cache!(path, cache) unless cache.nil? || !cache.active?
+        return @helper_cache.document(path) if @helper_cache.path_exists?(path)
+
+        PuppetLanguageServerSidecar.log_message(:debug, "[PuppetStringsHelper::file_documentation] Fetching documentation for #{path}")
+
+        PuppetLanguageServerSidecar::PuppetStringsHelper.setup_yard!
+
+        # For now, assume a single file path
+        search_patterns = [path]
+
+        # Format the arguments to YARD
+        args = ['doc']
+        args << '--no-output'
+        args << '--quiet'
+        args << '--no-stats'
+        args << '--no-progress'
+        args << '--no-save'
+        args << '--api public'
+        args << '--api private'
+        args << '--no-api'
+        args += search_patterns
+
+        # Run YARD
+        ::YARD::CLI::Yardoc.run(*args)
+
+        # Populate the documentation cache from the YARD information
+        @helper_cache.populate_from_yard_registry!
+
+        # Save to the permanent cache
+        @helper_cache.save_to_sidecar_cache(path, cache) unless cache.nil? || !cache.active?
+
+        # Return the documentation details
+        @helper_cache.document(path)
+      end
+    end
   end
 
   class FileDocumentationCache

spec/languageserver-sidecar/fixtures/real_agent/cache/.gitignore

@@ -0,0 +1,8 @@
+# These can be accidentally created during testing/development
+client_data/
+client_yaml/
+clientbucket/
+locales/
+preview/
+reports/
+state/

spec/languageserver-sidecar/fixtures/valid_module_workspace/lib/puppet/functions/fixture_pup4_function.rb

@@ -1,8 +1,31 @@
 # Example function using the Puppet 4 API in a module
 # This should be loaded as global namespace function
 Puppet::Functions.create_function(:fixture_pup4_function) do
-  # @return [Array<String>]
-  def fixture_pup4_function
-    'fixture_pup4_function result'
+  dispatch :method1 do
+    param 'String', :a_string
+    optional_block_param :block
+    return_type 'Array<String>'
+  end
+
+  # Does things with numbers
+  # @param an_integer The first number.
+  # @param values_to_average Zero or more additional numbers.
+  # @return [Array] Nothing useful
+  # @example Subtracting two arrays.
+  #   fixture_pup4_function(3, 2, 1) => ['Hello']
+  dispatch :method2 do
+    param 'Integer', :an_integer
+    optional_repeated_param 'Numeric', :values_to_average
+    return_type 'Array<String>'
+  end
+
+  def method1(a_string, &block)
+    ['fixture_pup4_function result']
+  end
+
+  def method2(an_integer, *values_to_average)
+    ['fixture_pup4_function result']
   end
 end
+
+# Note that method1 has no documentation.  This is for testing default documentation

spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/featureflag_puppetstrings/puppet_strings_helper_spec.rb

@@ -0,0 +1,230 @@
+require 'spec_helper'
+
+describe 'PuppetLanguageServerSidecar with Feature Flag puppetstrings' do
+  before(:each) do
+    skip('Puppet Strings is not available') if Gem::Specification.select { |item| item.name.casecmp('puppet-strings') }.count.zero?
+    skip('Puppet 6.0.0 or above is required') unless Gem::Version.new(Puppet.version) >= Gem::Version.new('6.0.0')
+
+    # Load files based on feature flags
+    ['puppet_strings_helper', 'puppet_strings_monkey_patches'].each do |lib|
+      require "puppet-languageserver-sidecar/#{lib}"
+    end
+  end
+
+  describe 'PuppetLanguageServerSidecar::PuppetStringsHelper' do
+    let(:subject) { PuppetLanguageServerSidecar::PuppetStringsHelper::Helper.new }
+    let(:cache) { nil }
+
+    # Classes
+    context 'Given a Puppet Class' do
+      let(:fixture_filepath) { File.join($fixtures_dir, 'real_agent', 'environments', 'testfixtures', 'modules', 'defaultmodule', 'manifests', 'init.pp' ) }
+
+      it 'should parse the file metadata correctly' do
+        result = subject.file_documentation(fixture_filepath, cache)
+
+        # There is only one class in the test fixture file
+        expect(result.classes.count).to eq(1)
+        item = result.classes[0]
+
+        # Check base methods
+        expect(item.key).to eq('defaultmodule')
+        expect(item.line).to eq(8)
+        expect(item.char).to be_nil
+        expect(item.length).to be_nil
+        expect(item.source).to eq(fixture_filepath)
+        # Check class specific methods
+        expect(item.doc).to match(/This is an example of how to document a Puppet class/)
+        # Check the class parameters
+        expect(item.parameters.count).to eq(2)
+        param = item.parameters['first']
+        expect(param[:doc]).to eq('The first parameter for this class.')
+        expect(param[:type]).to eq('String')
+        param = item.parameters['second']
+        expect(param[:doc]).to eq('The second parameter for this class.')
+        expect(param[:type]).to eq('Integer')
+      end
+    end
+
+    context 'Given a Puppet Defined Type' do
+      let(:fixture_filepath) { File.join($fixtures_dir, 'real_agent', 'environments', 'testfixtures', 'modules', 'defaultmodule', 'manifests', 'definedtype.pp' ) }
+
+      it 'should parse the file metadata correctly' do
+        result = subject.file_documentation(fixture_filepath, cache)
+
+        # There is only one class in the test fixture file
+        expect(result.classes.count).to eq(1)
+        item = result.classes[0]
+
+        # Check base methods
+        expect(item.key).to eq('defaultdefinedtype')
+        expect(item.line).to eq(6)
+        expect(item.char).to be_nil
+        expect(item.length).to be_nil
+        expect(item.source).to eq(fixture_filepath)
+        # Check class specific methods
+        expect(item.doc).to match(/This is an example of how to document a defined type./)
+        # Check the class parameters
+        expect(item.parameters.count).to eq(2)
+        param = item.parameters['ensure']
+        expect(param[:doc]).to eq('Ensure parameter documentation.')
+        expect(param[:type]).to eq('Any')
+        param = item.parameters['param2']
+        expect(param[:doc]).to eq('param2 documentation.')
+        expect(param[:type]).to eq('String')
+      end
+    end
+
+    # Functions
+    context 'Given a Ruby Puppet 3 API Function' do
+      let(:fixture_filepath) { File.join($fixtures_dir, 'real_agent', 'cache', 'lib', 'puppet', 'parser', 'functions', 'default_cache_function.rb' ) }
+
+      it 'should parse the file metadata correctly' do
+        result = subject.file_documentation(fixture_filepath, cache)
+
+        # There is only one function in the test fixture file
+        expect(result.functions.count).to eq(1)
+        item = result.functions[0]
+
+        # Check base methods
+        expect(item.key).to eq('default_cache_function')
+        expect(item.line).to eq(2)
+        expect(item.char).to eq(12)
+        expect(item.length).to eq(23)
+        expect(item.source).to eq(fixture_filepath)
+        # Check function specific methods
+        expect(item.doc).to match(/A function that should appear in the list of default functions/)
+        expect(item.function_version).to eq(3)
+        # Check the function signatures
+        expect(item.signatures.count).to eq(1)
+        sig = item.signatures[0]
+        expect(sig.doc).to match(/A function that should appear in the list of default functions/)
+        expect(sig.key).to eq('default_cache_function()')
+        expect(sig.return_types).to eq(['Any'])
+        # Check the function signature parameters
+        expect(sig.parameters.count).to eq(0)
+      end
+    end
+
+    context 'Given a Ruby Puppet 4 API Function' do
+      let(:fixture_filepath) { File.join($fixtures_dir, 'valid_module_workspace', 'lib', 'puppet', 'functions', 'fixture_pup4_function.rb') }
+
+      it 'should parse the file metadata correctly' do
+        result = subject.file_documentation(fixture_filepath, cache)
+
+        # There is only one function in the test fixture file
+        expect(result.functions.count).to eq(1)
+        item = result.functions[0]
+        # Check base methods
+        expect(item.key).to eq('fixture_pup4_function')
+        expect(item.line).to eq(3)
+        expect(item.char).to eq(34)
+        expect(item.length).to eq(22)
+        expect(item.source).to eq(fixture_filepath)
+        # Check function specific methods
+        expect(item.doc).to match(/Example function using the Puppet 4 API in a module/)
+        expect(item.function_version).to eq(4)
+        # Check the function signatures
+        expect(item.signatures.count).to eq(2)
+
+        # First signature - No yard documentation
+        sig = item.signatures[0]
+        expect(sig.doc).to eq('')
+        expect(sig.key).to eq('fixture_pup4_function(String $a_string, Optional[Callable] &$block)')
+        expect(sig.return_types).to eq(['Array<String>'])
+        # Check the function signature parameters
+        expect(sig.parameters.count).to eq(2)
+        sig_param = sig.parameters[0]
+        expect(sig_param.name).to eq('a_string')
+        expect(sig_param.doc).to eq('')
+        expect(sig_param.types).to eq(['String'])
+        sig_param = sig.parameters[1]
+        expect(sig_param.name).to eq('&block')
+        expect(sig_param.doc).to eq('')
+        expect(sig_param.types).to eq(['Optional[Callable]'])
+
+        # Second signature - Full yard documentation
+        sig = item.signatures[1]
+        expect(sig.doc).to eq('Does things with numbers')
+        expect(sig.key).to eq('fixture_pup4_function(Integer $an_integer, Optional[Numeric] *$values_to_average)')
+        expect(sig.return_types).to eq(['Array<String>'])
+        # Check the function signature parameters
+        expect(sig.parameters.count).to eq(2)
+        sig_param = sig.parameters[0]
+        expect(sig_param.name).to eq('an_integer')
+        expect(sig_param.doc).to eq('The first number.')
+        expect(sig_param.types).to eq(['Integer'])
+        sig_param = sig.parameters[1]
+        expect(sig_param.name).to eq('*values_to_average')
+        expect(sig_param.doc).to eq('Zero or more additional numbers.')
+        expect(sig_param.types).to eq(['Optional[Numeric]'])
+      end
+    end
+
+    context 'Given a Puppet Language Function' do
+      let(:fixture_filepath) { File.join($fixtures_dir, 'valid_module_workspace', 'functions', 'modulefunc.pp') }
+
+      it 'should parse the file metadata correctly' do
+        result = subject.file_documentation(fixture_filepath, cache)
+
+        # There is only one function in the test fixture file
+        expect(result.functions.count).to eq(1)
+        item = result.functions[0]
+        # Check base methods
+        expect(item.key).to eq('valid::modulefunc')
+        expect(item.line).to eq(7)
+        expect(item.char).to be_nil
+        expect(item.length).to be_nil
+        expect(item.source).to eq(fixture_filepath)
+        # Check function specific methods
+        expect(item.doc).to match(/An example puppet function in a module, as opposed to a ruby custom function/)
+        expect(item.function_version).to eq(4)
+        # Check the function signatures
+        expect(item.signatures.count).to eq(1)
+        sig = item.signatures[0]
+        expect(sig.doc).to match(/An example puppet function in a module, as opposed to a ruby custom function/)
+        expect(sig.key).to eq('valid::modulefunc(Variant[String, Boolean] $p1)')
+        expect(sig.return_types).to eq(['String'])
+        # Check the function signature parameters
+        expect(sig.parameters.count).to eq(1)
+        sig_param = sig.parameters[0]
+        expect(sig_param.name).to eq('p1')
+        expect(sig_param.doc).to eq('The first parameter for this function.')
+        expect(sig_param.types).to eq(['Variant[String, Boolean]'])
+      end
+    end
+
+    # Types
+    context 'Given a Puppet Custom Type' do
+      let(:fixture_filepath) { File.join($fixtures_dir, 'real_agent', 'cache', 'lib', 'puppet', 'type', 'default_type.rb' ) }
+
+      it 'should parse the file metadata correctly' do
+        result = subject.file_documentation(fixture_filepath, cache)
+
+        # There is only one type in the test fixture file
+        expect(result.types.count).to eq(1)
+        item = result.types[0]
+
+        # Check base methods
+        expect(item.key).to eq('default_type')
+        expect(item.line).to eq(1)
+        expect(item.char).to be_nil
+        expect(item.length).to be_nil
+        expect(item.source).to eq(fixture_filepath)
+        # Check type specific methods
+        expect(item.doc).to match(/Sets the global defaults for all printers on the system./)
+        # Check the type attributes
+        expect(item.attributes.count).to eq(2)
+        param = item.attributes['ensure']
+        expect(param[:doc]).to eq('The basic property that the resource should be in.')
+        expect(param[:type]).to eq(:property)
+        expect(param[:isnamevar?]).to be_nil
+        expect(param[:required?]).to be_nil
+        param = item.attributes['name']
+        expect(param[:doc]).to eq('The name of the default_type.')
+        expect(param[:type]).to eq(:param)
+        expect(param[:isnamevar?]).to eq(true)
+        expect(param[:required?]).to be_nil
+      end
+    end
+  end
+end