Merge pull request #1204 from tvpartytonight/CODEMGMT-1457

(CODEMGMT-1457) Flip `deploy-spec` setting to `exclude-spec`

lib/r10k/action/deploy/environment.rb

@@ -44,7 +44,7 @@ def initialize(opts, argv, settings = {})
                 preload_environments: true
               },
               modules: {
-                deploy_spec: settings.dig(:deploy, :deploy_spec),
+                exclude_spec: settings.dig(:deploy, :exclude_spec),
                 requested_modules: [],
                 deploy_modules: @modules,
                 force: !@no_force, # force here is used to make it easier to reason about
@@ -238,7 +238,7 @@ def allowed_initialize_opts
                       modules: :self,
                       cachedir: :self,
                       'no-force': :self,
-                      'deploy-spec': :self,
+                      'exclude-spec': :self,
                       'generate-types': :self,
                       'puppet-path': :self,
                       'puppet-conf': :self,

lib/r10k/action/deploy/module.rb

@@ -38,7 +38,7 @@ def initialize(opts, argv, settings = {})
                 generate_types: @generate_types
               },
               modules: {
-                deploy_spec: settings.dig(:deploy, :deploy_spec),
+                exclude_spec: settings.dig(:deploy, :exclude_spec),
                 requested_modules: @argv.map.to_a,
                 # force here is used to make it easier to reason about
                 force: !@no_force
@@ -118,7 +118,7 @@ def visit_environment(environment)
         def allowed_initialize_opts
           super.merge(environment: true,
                       cachedir: :self,
-                      'deploy-spec': :self,
+                      'exclude-spec': :self,
                       'no-force': :self,
                       'generate-types': :self,
                       'puppet-path': :self,

lib/r10k/action/runner.rb

@@ -44,12 +44,12 @@ def setup_settings
 
         overrides = {}
         overrides[:cachedir] = @opts[:cachedir] if @opts.key?(:cachedir)
-        if @opts.key?(:'puppet-path') || @opts.key?(:'generate-types') || @opts.key?(:'deploy-spec') || @opts.key?(:'puppet-conf')
+        if @opts.key?(:'puppet-path') || @opts.key?(:'generate-types') || @opts.key?(:'exclude-spec') || @opts.key?(:'puppet-conf')
           overrides[:deploy] = {}
           overrides[:deploy][:puppet_path] = @opts[:'puppet-path'] if @opts.key?(:'puppet-path')
           overrides[:deploy][:puppet_conf] = @opts[:'puppet-conf'] if @opts.key?(:'puppet-conf')
           overrides[:deploy][:generate_types] = @opts[:'generate-types'] if @opts.key?(:'generate-types')
-          overrides[:deploy][:deploy_spec] = @opts[:'deploy-spec'] if @opts.key?(:'deploy-spec')
+          overrides[:deploy][:exclude_spec] = @opts[:'exclude-spec'] if @opts.key?(:'exclude-spec')
         end
 
         with_overrides = config_settings.merge(overrides) do |key, oldval, newval|

lib/r10k/cli/deploy.rb

@@ -24,7 +24,7 @@ def self.command
         option nil, :cachedir, 'Specify a cachedir, overriding the value in config', argument: :required
         flag nil, :'no-force', 'Prevent the overwriting of local module modifications'
         flag nil, :'generate-types', 'Run `puppet generate types` after updating an environment'
-        flag nil, :'deploy-spec', 'Deploy the spec dir alongside other module directories'
+        flag nil, :'exclude-spec', 'Exclude the module\'s spec dir from deployment'
         option nil, :'puppet-path', 'Path to puppet executable', argument: :required do |value, cmd|
           unless File.executable? value
             $stderr.puts "The specified puppet executable #{value} is not executable."

lib/r10k/module/base.rb

@@ -57,8 +57,8 @@ def initialize(title, dirname, args, environment=nil)
     @environment = environment
     @overrides = args.delete(:overrides) || {}
     @spec_deletable = true
-    @deploy_spec = args.delete(:deploy_spec)
-    @deploy_spec = @overrides[:modules].delete(:deploy_spec) if @overrides.dig(:modules, :deploy_spec)
+    @exclude_spec = args.delete(:exclude_spec)
+    @exclude_spec = @overrides[:modules].delete(:exclude_spec) if @overrides.dig(:modules, :exclude_spec)
     @origin = 'external' # Expect Puppetfile or R10k::Environment to set this to a specific value
 
     @requested_modules = @overrides.dig(:modules, :requested_modules) || []
@@ -71,9 +71,9 @@ def full_path
     path.to_s
   end
 
-  # Delete the spec dir unless @deploy_spec has been set to true or @spec_deletable is false
+  # Delete the spec dir unless @exclude_spec has been set to true or @spec_deletable is false
   def maybe_delete_spec_dir
-    unless @deploy_spec
+    if @exclude_spec
       if @spec_deletable
         delete_spec_dir
       else

lib/r10k/settings.rb

@@ -195,12 +195,12 @@ def self.deploy_settings
             end
           end
         }),
-        Definition.new(:deploy_spec, {
+        Definition.new(:exclude_spec, {
           :desc => "Whether or not to deploy the spec dir of a module. Defaults to false.",
           :default => false,
           :validate => lambda do |value|
             unless !!value == value
-              raise ArgumentError, "`deploy_spec` can only be a boolean value, not '#{value}'"
+              raise ArgumentError, "`exclude_spec` can only be a boolean value, not '#{value}'"
             end
           end
         })])

spec/unit/action/deploy/environment_spec.rb

@@ -59,8 +59,8 @@
       described_class.new({ 'github-app-key': '/nonexistent' }, [], {})
     end
 
-    it 'can accept a deploy-spec option' do
-      described_class.new({ :'deploy-spec' => true }, [], {})
+    it 'can accept a exclude-spec option' do
+      described_class.new({ :'exclude-spec' => true }, [], {})
     end
 
     describe "initializing errors" do

spec/unit/action/deploy/module_spec.rb

@@ -54,8 +54,8 @@
       described_class.new({ 'github-app-key': '/nonexistent' }, [], {})
     end
 
-    it 'can accept a deploy-spec option' do
-      described_class.new({ :'deploy-spec' => true }, [], {})
+    it 'can accept a exclude-spec option' do
+      described_class.new({ :'exclude-spec' => true }, [], {})
     end
   end
 

spec/unit/module/base_spec.rb

@@ -42,27 +42,27 @@
       allow(logger).to receive(:info).with(any_args)
     end
 
-    it 'removes the spec directory' do
+    it 'does not remove the spec directory by default' do
       FileUtils.mkdir_p(spec_path)
       m = described_class.new(title, dirname, {})
       m.maybe_delete_spec_dir
-      expect(Dir.exist?(spec_path)).to eq false
+      expect(Dir.exist?(spec_path)).to eq true
     end
 
     it 'detects a symlink and deletes the target' do
       Dir.mkdir(dirname + module_name)
       target_dir = Dir.mktmpdir
       FileUtils.ln_s(target_dir, spec_path)
-      m = described_class.new(title, dirname, {})
+      m = described_class.new(title, dirname, {exclude_spec: true})
       m.maybe_delete_spec_dir
       expect(Dir.exist?(target_dir)).to eq false
     end
 
-    it 'does not remove the spec directory if deploy_spec is set' do
+    it 'removes the spec directory if exclude_spec is set' do
       FileUtils.mkdir_p(spec_path)
-      m = described_class.new(title, dirname, {deploy_spec: true})
+      m = described_class.new(title, dirname, {exclude_spec: true})
       m.maybe_delete_spec_dir
-      expect(Dir.exist?(spec_path)).to eq true
+      expect(Dir.exist?(spec_path)).to eq false
     end
 
     it 'does not remove the spec directory if spec_deletable is false' do

spec/unit/module/forge_spec.rb

@@ -175,12 +175,12 @@
       let(:spec_path) { dirname + module_name + 'spec' }
       subject { described_class.new(title, dirname, {}) }
 
-      it 'defaults to deleting the spec dir' do
+      it 'defaults to keeping the spec dir' do
         FileUtils.mkdir_p(spec_path)
         expect(subject).to receive(:status).and_return(:absent)
         expect(subject).to receive(:install)
         subject.sync
-        expect(Dir.exist?(spec_path)).to eq false
+        expect(Dir.exist?(spec_path)).to eq true
       end
     end
 

spec/unit/module/git_spec.rb

@@ -97,12 +97,12 @@
     let(:spec_path) { dirname + module_name + 'spec' }
     subject { described_class.new(title, dirname, {}) }
 
-    it 'defaults to deleting the spec dir' do
+    it 'defaults to keeping the spec dir' do
       FileUtils.mkdir_p(spec_path)
       allow(mock_repo).to receive(:resolve).with('master').and_return('abc123')
       allow(mock_repo).to receive(:sync)
       subject.sync
-      expect(Dir.exist?(spec_path)).to eq false
+      expect(Dir.exist?(spec_path)).to eq true
     end
   end
 

spec/unit/module/svn_spec.rb

@@ -127,13 +127,13 @@
     let(:spec_path) { dirname + module_name + 'spec' }
     subject { described_class.new(title, dirname, {}) }
 
-    it 'gets deleted by default' do
+    it 'is kept by default' do
 
       FileUtils.mkdir_p(spec_path)
       expect(subject).to receive(:status).and_return(:absent)
       expect(subject).to receive(:install).and_return(nil)
       subject.sync
-      expect(Dir.exist?(spec_path)).to eq false
+      expect(Dir.exist?(spec_path)).to eq true
     end
   end
 

spec/unit/module_spec.rb

@@ -91,15 +91,15 @@
           expect(obj).to be_a_kind_of(R10K::Module::Base)
         }.not_to raise_error
       end
-      describe 'the deploy_spec setting' do
-        it 'sets the deploy_spec instance variable' do
-          obj = R10K::Module.new(name, '/modulepath', options.merge({deploy_spec: true}))
-          expect(obj.instance_variable_get("@deploy_spec")).to eq(true)
+      describe 'the exclude_spec setting' do
+        it 'sets the exclude_spec instance variable' do
+          obj = R10K::Module.new(name, '/modulepath', options.merge({exclude_spec: true}))
+          expect(obj.instance_variable_get("@exclude_spec")).to eq(true)
         end
         it 'can be overridden by the overrides map' do
-          options = options.merge({deploy_spec: false, overrides: {modules: {deploy_spec: true}}})
+          options = options.merge({exclude_spec: false, overrides: {modules: {exclude_spec: true}}})
           obj = R10K::Module.new(name, '/modulepath', options)
-          expect(obj.instance_variable_get("@deploy_spec")).to eq(true)
+          expect(obj.instance_variable_get("@exclude_spec")).to eq(true)
         end
       end
     end

spec/unit/settings_spec.rb

@@ -95,21 +95,21 @@
   describe "deploy settings" do
     subject { described_class.deploy_settings }
 
-    describe 'deploy_spec' do
+    describe 'exclude_spec' do
       it 'is false by default' do
-        expect(subject.evaluate({})[:deploy_spec]).to eq(false)
+        expect(subject.evaluate({})[:exclude_spec]).to eq(false)
       end
       it 'can be set to true' do
-        expect(subject.evaluate({"deploy_spec" => true})[:deploy_spec]).to eq(true)
+        expect(subject.evaluate({"exclude_spec" => true})[:exclude_spec]).to eq(true)
       end
       it "raises an error for non-boolean values" do
         expect {
-          subject.evaluate({"deploy_spec" => 'invalid_string'})
+          subject.evaluate({"exclude_spec" => 'invalid_string'})
         }.to raise_error do |err|
           expect(err.message).to match(/Validation failed for 'deploy' settings group/)
           expect(err.errors.size).to eq 1
-          expect(err.errors[:deploy_spec]).to be_a_kind_of(ArgumentError)
-          expect(err.errors[:deploy_spec].message).to match(/`deploy_spec` can only be a boolean value, not 'invalid_string'/)
+          expect(err.errors[:exclude_spec]).to be_a_kind_of(ArgumentError)
+          expect(err.errors[:exclude_spec].message).to match(/`exclude_spec` can only be a boolean value, not 'invalid_string'/)
         end
       end
     end