(#759) Add reference spec tests for sensu_check JSON provider

Without this patch there are no spec tests for the sensu_check JSON provider.
This is problem because a reference is needed to specify the expected behavior
of all providers.

This patch implements a pattern of stubbing out the filesystem.  All reads and
writes in the provider itself are routed through the `read_file` and
`write_json_object` methods.  The RSpec tests then use rspec-mocks to stub out
the reads and set expectations on the output.

This reference may be applied to any provider using the `flush` method.  The
setter methods in the provider for each property are expected to update state in
an instance variable, conventionally named @property_flush but named @conf in
the sensu_check provider.  The flush method is responsible for writing out
@property_flush (@conf), which we intercept and set expectations on the data
provided.

N.B. There is a bug with Rspec where the expected and actual values of
multi-line strings will not have a nice diff output if the two strings disagree
on the presence of the trailing newline.  See
https://github.com/rspec/rspec-support/issues/70 for more information.  Because
of this issue in combination with the use of IO#puts in the write_output class
method, care should be taken with the examples to make sure the expected and
actual values agree on the trailing newline.  In this patch, the fixture data
for the expected output is chomp()'ed to match the string passed to
write_output().

Resolves #759

Gemfile

@@ -15,6 +15,7 @@ end
 group :development, :unit_tests do
   gem 'rake',                                             '< 11.0.0'
   gem 'rspec-puppet', '~> 2.5.0',                         :require => false
+  gem 'rspec-mocks',                                      :require => false
   gem 'puppetlabs_spec_helper', '>= 2.0.0',               :require => false
   gem 'puppet-lint', "~> 2.0",                            :require => false
   gem 'json', "~> 1.8.3",                                 :require => false

lib/puppet/provider/sensu_check/json.rb

@@ -11,19 +11,63 @@
 
   SENSU_CHECK_PROPERTIES = Puppet::Type.type(:sensu_check).validproperties.reject { |p| p == :ensure }
 
+  # read_file provides a well-known location for spec tests to intercept and
+  # stub out filesystem calls.  File.read itself is not stubbed out because
+  # File.read is called from many places.  This helper method affords precision
+  # to the spec examples.
+  #
+  # @param [String] fpath the fully qualified path to read.
+  #
+  # @return [String] the file content.
+  def self.read_file(fpath)
+    File.read(fpath)
+  end
+
+  # Passes through to .read_file
+  def read_file(fpath)
+    self.class.read_file(fpath)
+  end
+
+  # Write a string to a file.  Note, `puts` is used to write data which will
+  # insert a trailing newline if absent.
+  #
+  # @param [String] fpath the full qualified path to write.
+  #
+  # @param [String] data the data to write.
+  def self.write_output(fpath, data)
+    File.open(fpath, 'w') do |f|
+      f.puts(data)
+    end
+  end
+
+  # provide a well-known location for spec tests to intercept and stub out
+  # filesystem calls.
+  #
+  # @param [String] fpath the fully qualified path to read.
+  #
+  # @param [<Hash,Array>] obj The JSON object to write out to fpath.
+  #
+  # @return [String] the file content.
+  def self.write_json_object(fpath, obj)
+    write_output(fpath, JSON.pretty_generate(obj))
+  end
+
+  # Passes through to .write_json_object
+  def write_json_object(fpath, obj)
+    self.class.write_json_object(fpath, obj)
+  end
+
   def conf
     begin
-      @conf ||= JSON.parse(File.read(config_file))
+      @conf ||= JSON.parse(read_file(config_file))
     rescue
       @conf ||= {}
     end
   end
 
   def flush
     sort_properties!
-    File.open(config_file, 'w') do |f|
-      f.puts JSON.pretty_generate(conf)
-    end
+    write_json_object(config_file, conf)
   end
 
   def pre_create

spec/fixtures/unit/provider/sensu_check/json/mycheck_custom_input.json

@@ -0,0 +1,22 @@
+{
+  "checks": {
+    "remote_http": {
+      "command": "/opt/sensu/embedded/bin/check-http.rb -u http://:::address:::",
+      "foo": "bar",
+      "high_flap_threshold": 60,
+      "interval": 300,
+      "low_flap_threshold": 20,
+      "occurrences": 2,
+      "proxy_requests": {
+        "client_attributes": {
+          "subscriptions": "eval: value.include?(\"http\")"
+        }
+      },
+      "refresh": 600,
+      "standalone": false,
+      "subscribers": [
+        "roundrobin:poller"
+      ]
+    }
+  }
+}

spec/fixtures/unit/provider/sensu_check/json/mycheck_example_input.json

@@ -0,0 +1,21 @@
+{
+  "checks": {
+    "remote_http": {
+      "command": "/opt/sensu/embedded/bin/check-http.rb -u http://:::address:::",
+      "high_flap_threshold": 60,
+      "interval": 300,
+      "low_flap_threshold": 20,
+      "occurrences": 2,
+      "proxy_requests": {
+        "client_attributes": {
+          "subscriptions": "eval: value.include?(\"http\")"
+        }
+      },
+      "refresh": 600,
+      "standalone": false,
+      "subscribers": [
+        "roundrobin:poller"
+      ]
+    }
+  }
+}

spec/fixtures/unit/provider/sensu_check/json/mycheck_expected_output.json

@@ -0,0 +1,28 @@
+{
+  "checks": {
+    "remote_http": {
+      "boolval": true,
+      "command": "/opt/sensu/embedded/bin/check-http.rb -u http://:::address:::",
+      "foo": "bar",
+      "high_flap_threshold": 60,
+      "in_array": [
+        "foo",
+        "baz"
+      ],
+      "interval": 300,
+      "low_flap_threshold": 20,
+      "numval": 6,
+      "occurrences": 2,
+      "proxy_requests": {
+        "client_attributes": {
+          "subscriptions": "eval: value.include?(\"http\")"
+        }
+      },
+      "refresh": 600,
+      "standalone": false,
+      "subscribers": [
+        "roundrobin:poller"
+      ]
+    }
+  }
+}

spec/fixtures/unit/provider/sensu_check/json/mycheck_unsorted_input.json

@@ -0,0 +1,28 @@
+{
+  "checks": {
+    "remote_http": {
+      "command": "/opt/sensu/embedded/bin/check-http.rb -u http://:::address:::",
+      "high_flap_threshold": 60,
+      "interval": 300,
+      "occurrences": 2,
+      "proxy_requests": {
+        "client_attributes": {
+          "subscriptions": "eval: value.include?(\"http\")"
+        }
+      },
+      "refresh": 600,
+      "standalone": false,
+      "low_flap_threshold": 20,
+      "in_array": [
+        "foo",
+        "baz"
+      ],
+      "numval": 6,
+      "boolval": true,
+      "foo": "bar",
+      "subscribers": [
+        "roundrobin:poller"
+      ]
+    }
+  }
+}

spec/spec_helper.rb

@@ -12,6 +12,7 @@
 end
 
 RSpec.configure do |config|
+  config.mock_with :rspec
   config.hiera_config = 'spec/fixtures/hiera/hiera.yaml'
   config.before :each do
     # Ensure that we don't accidentally cache facts and environment between

spec/unit/provider/sensu_check/json_spec.rb

@@ -0,0 +1,177 @@
+require 'spec_helper'
+
+# The goal of the let methods are to wire up a provider into a harness used for
+# testing.  During Puppet runtime, there are multiple contexts a provider
+# operates within.  The two primary ones are enforcement; e.g. `puppet apply`
+# mode, and introspection, e.g. `puppet resource` mode.  During enforcement,
+# there is an associated resource modeled in the catalog.  During introspection,
+# a resource is initially absent and the provider provides information to
+# initialize the resource.
+#
+# Terminology used in the let helper methods.
+#
+# "type_id" refers to the Symbol identifying the Type.  e.g. :sensu_check
+#
+# "resource" is an instance of Puppet::Type.type(type) as it would exist in the
+# RAL during catalog application.  This resource contains the desired state
+# information, the properties and parameters specified in the Puppet DSL.
+#
+# "provider" is an instance of the provider class being tested.  In Puppet,
+# provider instances exist primarily in one of two states, either bound or not
+# bound to a resource.  Provider instances are not bound when the system is
+# being introspected, e.g. `puppet resource service` calls the `instances` class
+# method which will instantiate provider instances which have no associated
+# resource.  When applying a Puppet catalog, each provider is associated with
+# exactly one resource from the Puppet DSL.
+#
+# Because of this dual nature, providers must be careful when accessing
+# parameter data, e.g. `base_path`.  Since `base_path` is a parameter, it will
+# not be accessible in the context of self.instances and `puppet resource`,
+# because there is not a bound resource when discovering resources.
+#
+# When building a new provider with spec tests, start with `self.instances`,
+# because this approach exercises a provider with the minimal amount of state.
+# That is to say, the provider must be well-behaved when there is no associated
+# resource.
+#
+# property_hash or @property_hash is an instance variable describing the current
+# state of the resource as it exists on the target system.  Take care not to
+# confuse this with the data contained in the resource, which describes desired
+# state.
+#
+# property_flush or @property_flush is an instance variable used to modify the
+# system from the `flush` method.  Setter methods, one for each property of the
+# resource type, should modify @property_flush
+
+type_id = :sensu_check
+
+describe Puppet::Type.type(type_id).provider(:json) do
+  let(:catalog) { Puppet::Resource::Catalog.new }
+  let(:type) { Puppet::Type.type(type_id) }
+  # The title of the resource, for convenience
+  let(:title) { 'remote_http' }
+
+  # The default resource hash modeling the resource in a manifest.
+  let(:rsrc_hsh_base) do
+    { name: title, ensure: 'present' }
+  end
+  # Override this helper method in nested example groups
+  let(:rsrc_hsh_override) { {} }
+  # Combined resource hash.  Used to initialize @provider_hash via new()
+  let(:rsrc_hsh) { rsrc_hsh_base.merge(rsrc_hsh_override) }
+  # A provider with @property_hash initialized, but without a resource.
+  let(:bare_provider) { described_class.new(rsrc_hsh) }
+  # A resource bound to bare_provider.  This has the side-effect of associating
+  # the provider instance to a resource (bare_provider is no longer bare of a
+  # resource.)
+  let(:resource) { type.new(rsrc_hsh.merge(provider: bare_provider)) }
+  # A "harnessed" provider instance suitable for testing.  @property_hash is
+  # initialized and provider.resource returns a Resource.
+  let(:provider) do
+    resource.provider
+  end
+
+  context 'during catalog application' do
+    describe 'parameters (provide data)' do
+      describe '#name' do
+        subject { provider.name }
+        it { is_expected.to eq title }
+      end
+    end
+
+    # Properties modify the system.  Parameters add supporting data.
+    describe 'properties (take action)' do
+      describe 'when writing JSON data to the filesystem with #flush' do
+        describe '#custom' do
+          context 'with a pre-existing check definition' do
+            # An existing JSON file the provider will modify.
+            let(:input) do
+              File.read(my_fixture('mycheck_example_input.json'))
+            end
+            # Stub out the filesystem read with fixture data
+            before :each do
+              allow(provider).to receive(:read_file).and_return(input)
+            end
+
+            subject { provider.custom }
+
+            context 'without custom configuration' do
+              it { is_expected.to eq({}) }
+            end
+            context 'with custom configuration' do
+              let(:input) do
+                File.read(my_fixture('mycheck_custom_input.json'))
+              end
+              it { is_expected.to eq({'foo' => 'bar'}) }
+            end
+          end
+        end
+
+        describe '#custom=' do
+          context 'with pre-existing configuration on the system' do
+            # An existing JSON file the provider will modify.
+            let(:input) do
+              File.read(my_fixture('mycheck_example_input.json'))
+            end
+
+            let(:expected_output) do
+              File.read(my_fixture('mycheck_expected_output.json'))
+            end
+
+            before :each do
+              # The fixed input for testing.  This is an expectation so a
+              # failure is triggered if the stub becomes mis-matched with the
+              # implemented behavior.
+              expect(provider).to receive(:read_file).and_return(input)
+            end
+
+            context 'with custom defined' do
+              # Example value for the custom property from the README
+              let(:custom) do
+                {
+                  'foo'      => 'bar',
+                  'numval'   => 6,
+                  'boolval'  => true,
+                  'in_array' => ['foo','baz'],
+                }
+              end
+
+              # The desired state from the catalog
+              let(:rsrc_hsh_override) { {custom: custom} }
+
+              it 'writes the configuration file as a JSON object' do
+                # TODO: Would be nice to make this a shared expectation
+                expect(provider).to receive(:write_json_object) do |fp, obj|
+                  expect(fp).to eq(provider.config_file)
+                  ex_out = JSON.parse(expected_output)
+                  check_def = ex_out['checks']['remote_http']
+                  # This gives a nice diff if there is an issue
+                  expect(obj['checks']['remote_http']).to eq(check_def)
+                  # This tests the complete configuration
+                  expect(obj).to eq(ex_out)
+                end
+
+                provider.custom = custom
+                provider.flush
+              end
+            end
+
+            context 'with unsorted input JSON' do
+              let(:input) do
+                File.read(my_fixture('mycheck_unsorted_input.json'))
+              end
+              it 'writes sorted JSON output' do
+                expect(described_class).to receive(:write_output) do |_, data|
+                  # Trailing newlines must match to get a nice diff
+                  # See: https://github.com/rspec/rspec-support/issues/70
+                  expect(data).to eq(expected_output.chomp)
+                end
+                provider.flush
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end