From 5664a9935434863467dc2ba3d6ec17f6f2f839f9 Mon Sep 17 00:00:00 2001
From: David Schmitt <david.schmitt@puppet.com>
Date: Fri, 9 Mar 2018 13:55:29 +0000
Subject: [PATCH 1/3] (PDK-508) Setup easy testing of test_module

By pulling in puppetlabs_spec_helper into the main project, we can run
tests for the `test_module` in the main test suite without friction.
This makes it easier and quicker to run this kind of tests.

See the next commits for usage.

This requires https://github.com/puppetlabs/puppetlabs_spec_helper/pull/234
---
 .dependency_decisions.yml | 7 +++++++
 .fixtures.yml             | 8 ++++++++
 .gitignore                | 3 +++
 Gemfile                   | 5 ++++-
 Rakefile                  | 5 ++++-
 spec/spec_helper.rb       | 4 ++++
 6 files changed, 30 insertions(+), 2 deletions(-)
 create mode 100644 .fixtures.yml

diff --git a/.dependency_decisions.yml b/.dependency_decisions.yml
index 265ddd01..78698538 100644
--- a/.dependency_decisions.yml
+++ b/.dependency_decisions.yml
@@ -89,3 +89,10 @@
     :why: https://github.com/masover/blankslate/blob/f0a73b80c34bc3ad94dc3195d12a227a8fe30019/MIT-LICENSE
     :versions: []
     :when: 2017-11-17 14:05:09.534340925 Z
+- - :license
+  - puppetlabs_spec_helper
+  - Apache 2.0
+  - :who: DavidS
+    :why: 
+    :versions: []
+    :when: 2018-03-09 18:04:29.175843919 Z
diff --git a/.fixtures.yml b/.fixtures.yml
new file mode 100644
index 00000000..bf4c5dc9
--- /dev/null
+++ b/.fixtures.yml
@@ -0,0 +1,8 @@
+# This file can be used to install module depdencies for unit testing
+# See https://github.com/puppetlabs/puppetlabs_spec_helper#using-fixtures for details
+---
+fixtures:
+  forge_modules:
+#     stdlib: "puppetlabs/stdlib"
+  symlinks:
+    test_module: "#{source_dir}/spec/fixtures/test_module"
diff --git a/.gitignore b/.gitignore
index 8eb3b06b..dedffc53 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,3 +10,6 @@
 
 # rspec failure tracking
 .rspec_status
+
+# puppetlabs_spec_helper automatic fixtures
+spec/fixtures/modules/test_module
diff --git a/Gemfile b/Gemfile
index ccc7855a..8b2fa581 100644
--- a/Gemfile
+++ b/Gemfile
@@ -15,6 +15,9 @@ group :tests do
   gem 'rubocop-rspec'
   gem 'rubocop'
   gem 'simplecov-console'
+  # the test gems required for module testing
+  gem 'puppetlabs_spec_helper', github: 'DavidS/puppetlabs_spec_helper', ref: 'refactor'
+  gem 'rspec-puppet'
 end
 
 group :development do
@@ -39,5 +42,5 @@ end
 if ENV['PUPPET_GEM_VERSION']
   gem 'puppet', *location_for(ENV['PUPPET_GEM_VERSION'])
 else
-  gem 'puppet', git: 'https://github.com/DavidS/puppet', ref: 'device-apply'
+  gem 'puppet', github: 'DavidS/puppet', ref: 'device-apply'
 end
diff --git a/Rakefile b/Rakefile
index bc1f5624..c4b4ccae 100644
--- a/Rakefile
+++ b/Rakefile
@@ -1,4 +1,5 @@
 require 'bundler/gem_tasks'
+require 'puppetlabs_spec_helper/tasks/fixtures'
 
 task :default => :spec
 
@@ -13,7 +14,9 @@ end
 require 'rspec/core/rake_task'
 
 RSpec::Core::RakeTask.new(:spec) do |t|
-  excludes = ['fixtures/**/*.rb']
+  Rake::Task[:spec_prep].invoke
+  # thanks to the fixtures/modules/ symlinks this needs to exclude fixture modules explicitely
+  excludes = ['fixtures/**/*.rb,fixtures/modules/*/**/*.rb']
   if RUBY_PLATFORM == 'java'
     excludes += ['acceptance/**/*.rb', 'integration/**/*.rb', 'puppet/resource_api/*_context_spec.rb', 'puppet/util/network_device/simple/device_spec.rb']
     t.rspec_opts = '--tag ~agent_test'
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 7eea1494..18da6220 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -31,6 +31,7 @@
 
 require 'bundler/setup'
 require 'puppet/resource_api'
+require 'puppetlabs_spec_helper/module_spec_helper'
 
 RSpec.configure do |config|
   # Enable flags like --only-failures and --next-failure
@@ -42,4 +43,7 @@
   config.expect_with :rspec do |c|
     c.syntax = :expect
   end
+
+  # override legacy default from puppetlabs_spec_helper
+  config.mock_with :rspec
 end

From 965840a2a2a7edc47a98cac192a06998eac34175 Mon Sep 17 00:00:00 2001
From: David Schmitt <david.schmitt@puppet.com>
Date: Mon, 12 Mar 2018 11:16:38 +0000
Subject: [PATCH 2/3] (PDK-508) Test class and provider straight from the
 templates

This is a separate commit to make it easier to see what was changed
for the test.
---
 spec/classes/autorequire_cycle_spec.rb        |  5 ++
 spec/fixtures/manifests/site.pp               |  0
 .../test_autorequire/test_autorequire.rb      | 30 +++++++++++
 .../lib/puppet/type/test_autorequire.rb       | 21 ++++++++
 .../manifests/autorequire_cycle.pp            | 10 ++++
 .../spec/classes/autorequire_cycle_spec.rb    | 11 ++++
 .../test_autorequire/test_autorequire_spec.rb | 50 +++++++++++++++++++
 7 files changed, 127 insertions(+)
 create mode 100644 spec/classes/autorequire_cycle_spec.rb
 create mode 100644 spec/fixtures/manifests/site.pp
 create mode 100644 spec/fixtures/test_module/lib/puppet/provider/test_autorequire/test_autorequire.rb
 create mode 100644 spec/fixtures/test_module/lib/puppet/type/test_autorequire.rb
 create mode 100644 spec/fixtures/test_module/manifests/autorequire_cycle.pp
 create mode 100644 spec/fixtures/test_module/spec/classes/autorequire_cycle_spec.rb
 create mode 100644 spec/fixtures/test_module/spec/unit/puppet/provider/test_autorequire/test_autorequire_spec.rb

diff --git a/spec/classes/autorequire_cycle_spec.rb b/spec/classes/autorequire_cycle_spec.rb
new file mode 100644
index 00000000..990945d9
--- /dev/null
+++ b/spec/classes/autorequire_cycle_spec.rb
@@ -0,0 +1,5 @@
+require 'spec_helper'
+
+RSpec.describe 'test_module::autorequire_cycle' do
+  it { is_expected.to compile }
+end
diff --git a/spec/fixtures/manifests/site.pp b/spec/fixtures/manifests/site.pp
new file mode 100644
index 00000000..e69de29b
diff --git a/spec/fixtures/test_module/lib/puppet/provider/test_autorequire/test_autorequire.rb b/spec/fixtures/test_module/lib/puppet/provider/test_autorequire/test_autorequire.rb
new file mode 100644
index 00000000..a0c39e00
--- /dev/null
+++ b/spec/fixtures/test_module/lib/puppet/provider/test_autorequire/test_autorequire.rb
@@ -0,0 +1,30 @@
+require 'puppet/resource_api'
+require 'puppet/resource_api/simple_provider'
+
+# Implementation for the test_autorequire type using the Resource API.
+class Puppet::Provider::TestAutorequire::TestAutorequire < Puppet::ResourceApi::SimpleProvider
+  def get(_context)
+    [
+      {
+        name: 'foo',
+        ensure: :present,
+      },
+      {
+        name: 'bar',
+        ensure: :present,
+      },
+    ]
+  end
+
+  def create(context, name, should)
+    context.notice("Creating '#{name}' with #{should.inspect}")
+  end
+
+  def update(context, name, should)
+    context.notice("Updating '#{name}' with #{should.inspect}")
+  end
+
+  def delete(context, name)
+    context.notice("Deleting '#{name}'")
+  end
+end
diff --git a/spec/fixtures/test_module/lib/puppet/type/test_autorequire.rb b/spec/fixtures/test_module/lib/puppet/type/test_autorequire.rb
new file mode 100644
index 00000000..ab451f2d
--- /dev/null
+++ b/spec/fixtures/test_module/lib/puppet/type/test_autorequire.rb
@@ -0,0 +1,21 @@
+require 'puppet/resource_api'
+
+Puppet::ResourceApi.register_type(
+  name: 'test_autorequire',
+  docs: <<-EOS,
+      This type provides Puppet with the capabilities to manage ...
+    EOS
+  features: [],
+  attributes:   {
+    ensure:      {
+      type:    'Enum[present, absent]',
+      desc:    'Whether this resource should be present or absent on the target system.',
+      default: 'present',
+    },
+    name:        {
+      type:      'String',
+      desc:      'The name of the resource you want to manage.',
+      behaviour: :namevar,
+    },
+  },
+)
diff --git a/spec/fixtures/test_module/manifests/autorequire_cycle.pp b/spec/fixtures/test_module/manifests/autorequire_cycle.pp
new file mode 100644
index 00000000..3363e263
--- /dev/null
+++ b/spec/fixtures/test_module/manifests/autorequire_cycle.pp
@@ -0,0 +1,10 @@
+# test_module::autorequire_cycle
+#
+# A description of what this class does
+#
+# @summary A short summary of the purpose of this class
+#
+# @example
+#   include test_module::autorequire_cycle
+class test_module::autorequire_cycle {
+}
diff --git a/spec/fixtures/test_module/spec/classes/autorequire_cycle_spec.rb b/spec/fixtures/test_module/spec/classes/autorequire_cycle_spec.rb
new file mode 100644
index 00000000..27cc17af
--- /dev/null
+++ b/spec/fixtures/test_module/spec/classes/autorequire_cycle_spec.rb
@@ -0,0 +1,11 @@
+require 'spec_helper'
+
+describe 'test_module::autorequire_cycle' do
+  on_supported_os.each do |os, os_facts|
+    context "on #{os}" do
+      let(:facts) { os_facts }
+
+      it { is_expected.to compile }
+    end
+  end
+end
diff --git a/spec/fixtures/test_module/spec/unit/puppet/provider/test_autorequire/test_autorequire_spec.rb b/spec/fixtures/test_module/spec/unit/puppet/provider/test_autorequire/test_autorequire_spec.rb
new file mode 100644
index 00000000..b97795f5
--- /dev/null
+++ b/spec/fixtures/test_module/spec/unit/puppet/provider/test_autorequire/test_autorequire_spec.rb
@@ -0,0 +1,50 @@
+require 'spec_helper'
+
+# TODO: needs some cleanup/helper to avoid this misery
+module Puppet::Provider::TestAutorequire; end
+require 'puppet/provider/test_autorequire/test_autorequire'
+
+RSpec.describe Puppet::Provider::TestAutorequire::TestAutorequire do
+  subject(:provider) { described_class.new }
+
+  let(:context) { instance_double('Puppet::ResourceApi::BaseContext', 'context') }
+
+  describe '#get' do
+    it 'processes resources' do
+      expect(provider.get(context)).to eq [
+        {
+          name: 'foo',
+          ensure: :present,
+        },
+        {
+          name: 'bar',
+          ensure: :present,
+        },
+      ]
+    end
+  end
+
+  describe 'create(context, name, should)' do
+    it 'creates the resource' do
+      expect(context).to receive(:notice).with(%r{\ACreating 'a'})
+
+      provider.create(context, 'a', name: 'a', ensure: 'present')
+    end
+  end
+
+  describe 'update(context, name, should)' do
+    it 'updates the resource' do
+      expect(context).to receive(:notice).with(%r{\AUpdating 'foo'})
+
+      provider.update(context, 'foo', name: 'foo', ensure: 'present')
+    end
+  end
+
+  describe 'delete(context, name, should)' do
+    it 'deletes the resource' do
+      expect(context).to receive(:notice).with(%r{\ADeleting 'foo'})
+
+      provider.delete(context, 'foo')
+    end
+  end
+end

From 8cbec7fd45c3b24db56e282a1b31284472eb3013 Mon Sep 17 00:00:00 2001
From: David Schmitt <david.schmitt@puppet.com>
Date: Mon, 12 Mar 2018 11:14:31 +0000
Subject: [PATCH 3/3] (PDK-508) implement autorequire and friends

The spec/classes/autorequire_cycle_spec.rb test shows that the
autorelations implementation of the Resource API does build relations.
Since rspec-puppet doesn't provide matching on compile errors, this
builds a test case that compiles when make_cycle is false, thereby
proving that there is no error in the manifest, but fails with a
dependency cycle error when make_cycle is set to true.
---
 lib/puppet/resource_api.rb                    | 18 +++++++
 spec/classes/autorequire_cycle_spec.rb        | 11 +++-
 .../lib/puppet/type/test_autorequire.rb       |  8 +++
 .../manifests/autorequire_cycle.pp            | 19 +++++--
 .../spec/classes/autorequire_cycle_spec.rb    | 15 +++---
 spec/puppet/resource_api_spec.rb              | 51 +++++++++++++++++++
 6 files changed, 112 insertions(+), 10 deletions(-)

diff --git a/lib/puppet/resource_api.rb b/lib/puppet/resource_api.rb
index 59e70969..b10169a9 100644
--- a/lib/puppet/resource_api.rb
+++ b/lib/puppet/resource_api.rb
@@ -248,6 +248,24 @@ def my_provider
       def context
         self.class.context
       end
+
+      [:autorequire, :autobefore, :autosubscribe, :autonotify].each do |auto|
+        next unless definition[auto]
+
+        definition[auto].each do |type, values|
+          Puppet.debug("Registering #{auto} for #{type}: #{values.inspect}")
+          send(auto, type.downcase.to_sym) do
+            [values].flatten.map do |v|
+              match = %r{\A\$(.*)\Z}.match(v) if v.is_a? String
+              if match.nil?
+                v
+              else
+                self[match[1].to_sym]
+              end
+            end
+          end
+        end
+      end
     end
   end
   module_function :register_type
diff --git a/spec/classes/autorequire_cycle_spec.rb b/spec/classes/autorequire_cycle_spec.rb
index 990945d9..c5527fbb 100644
--- a/spec/classes/autorequire_cycle_spec.rb
+++ b/spec/classes/autorequire_cycle_spec.rb
@@ -1,5 +1,14 @@
 require 'spec_helper'
 
 RSpec.describe 'test_module::autorequire_cycle' do
-  it { is_expected.to compile }
+  context 'with make_cycle => false' do
+    let(:params) { { make_cycle: false } }
+
+    it { is_expected.to compile }
+  end
+  context 'with make_cycle => true' do
+    let(:params) { { make_cycle: true } }
+
+    it { is_expected.not_to compile }
+  end
 end
diff --git a/spec/fixtures/test_module/lib/puppet/type/test_autorequire.rb b/spec/fixtures/test_module/lib/puppet/type/test_autorequire.rb
index ab451f2d..47766e14 100644
--- a/spec/fixtures/test_module/lib/puppet/type/test_autorequire.rb
+++ b/spec/fixtures/test_module/lib/puppet/type/test_autorequire.rb
@@ -17,5 +17,13 @@
       desc:      'The name of the resource you want to manage.',
       behaviour: :namevar,
     },
+    target:      {
+      type:      'String',
+      desc:      'The resource to autorequire.',
+      behaviour: :parameter,
+    },
+  },
+  autorequire: {
+    test_autorequire: '$target',
   },
 )
diff --git a/spec/fixtures/test_module/manifests/autorequire_cycle.pp b/spec/fixtures/test_module/manifests/autorequire_cycle.pp
index 3363e263..9d9bf92f 100644
--- a/spec/fixtures/test_module/manifests/autorequire_cycle.pp
+++ b/spec/fixtures/test_module/manifests/autorequire_cycle.pp
@@ -1,10 +1,23 @@
 # test_module::autorequire_cycle
 #
-# A description of what this class does
+# This class is used to test autorequires.
+# With make_cycle set to false, this should compile without errors or cycles. When make_cycle is set to true, autorequires will be used to
+# construct a dependency cycle. This makes it possible to test exactly the function of the autorequires implementation.
 #
-# @summary A short summary of the purpose of this class
+# @summary This class is used to test autorequires.
 #
 # @example
 #   include test_module::autorequire_cycle
-class test_module::autorequire_cycle {
+class test_module::autorequire_cycle (
+  Boolean $make_cycle
+) {
+  test_autorequire { "a":
+    target => "b",
+  }
+  test_autorequire { "b":
+    target => "c",
+  }
+  test_autorequire { "c":
+    target => $make_cycle ? { true => "a", false => undef },
+  }
 }
diff --git a/spec/fixtures/test_module/spec/classes/autorequire_cycle_spec.rb b/spec/fixtures/test_module/spec/classes/autorequire_cycle_spec.rb
index 27cc17af..c5527fbb 100644
--- a/spec/fixtures/test_module/spec/classes/autorequire_cycle_spec.rb
+++ b/spec/fixtures/test_module/spec/classes/autorequire_cycle_spec.rb
@@ -1,11 +1,14 @@
 require 'spec_helper'
 
-describe 'test_module::autorequire_cycle' do
-  on_supported_os.each do |os, os_facts|
-    context "on #{os}" do
-      let(:facts) { os_facts }
+RSpec.describe 'test_module::autorequire_cycle' do
+  context 'with make_cycle => false' do
+    let(:params) { { make_cycle: false } }
 
-      it { is_expected.to compile }
-    end
+    it { is_expected.to compile }
+  end
+  context 'with make_cycle => true' do
+    let(:params) { { make_cycle: true } }
+
+    it { is_expected.not_to compile }
   end
 end
diff --git a/spec/puppet/resource_api_spec.rb b/spec/puppet/resource_api_spec.rb
index e3843f5d..c8194cbd 100644
--- a/spec/puppet/resource_api_spec.rb
+++ b/spec/puppet/resource_api_spec.rb
@@ -90,6 +90,18 @@
             desc: 'a optional string value',
           },
         },
+        autorequire: {
+          var: '$test_string',
+        },
+        autobefore: {
+          const: 'value',
+        },
+        autosubscribe: {
+          list: %w[foo bar],
+        },
+        autonotify: {
+          mixed: [10, '$test_integer'],
+        },
       }
     end
 
@@ -101,6 +113,45 @@
       it { is_expected.not_to be_nil }
       it { expect(type.properties.first.doc).to match %r{the description} }
       it { expect(type.properties.first.name).to eq :test_string }
+
+      def extract_values(function)
+        result = []
+        type.send(function) do |_type, values|
+          # rely on the fact that the resource api is doing `self[]` internally to find the value
+          # see https://github.com/puppetlabs/puppet/blob/9f2c143962803a72c68f35be3462944e851bcdce/lib/puppet/type.rb#L2143
+          # for details
+          result += { test_string: 'foo', test_integer: 100 }.instance_eval(&values)
+        end
+        result
+      end
+
+      describe 'autorequire' do
+        it('yields the block for `var`') { expect { |b| type.eachautorequire(&b) }.to yield_with_args(:var, be_a(Proc)) }
+        it 'the yielded block returns the `test_string` value' do
+          expect(extract_values(:eachautorequire)).to eq ['foo']
+        end
+      end
+
+      describe 'autobefore' do
+        it('yields the block for `const`') { expect { |b| type.eachautobefore(&b) }.to yield_with_args(:const, be_a(Proc)) }
+        it('the yielded block returns the constant "value"') do
+          expect(extract_values(:eachautobefore)).to eq ['value']
+        end
+      end
+
+      describe 'autosubscribe' do
+        it('yields the block for `list`') { expect { |b| type.eachautosubscribe(&b) }.to yield_with_args(:list, be_a(Proc)) }
+        it('the yielded block returns the multiple values') do
+          expect(extract_values(:eachautosubscribe)).to eq %w[foo bar]
+        end
+      end
+
+      describe 'autonotify' do
+        it('yields the block for `mixed`') { expect { |b| type.eachautonotify(&b) }.to yield_with_args(:mixed, be_a(Proc)) }
+        it('the yielded block returns multiple integer values') do
+          expect(extract_values(:eachautonotify)).to eq [10, 100]
+        end
+      end
     end
 
     describe 'an instance of this type' do