Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(CODEMGMT-1457) Flip deploy-spec setting to exclude-spec #1204

Merged
merged 1 commit into from
Aug 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/r10k/action/deploy/environment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions lib/r10k/action/deploy/module.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions lib/r10k/action/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
2 changes: 1 addition & 1 deletion lib/r10k/cli/deploy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
8 changes: 4 additions & 4 deletions lib/r10k/module/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) || []
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/r10k/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
})])
Expand Down
4 changes: 2 additions & 2 deletions spec/unit/action/deploy/environment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions spec/unit/action/deploy/module_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 6 additions & 6 deletions spec/unit/module/base_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions spec/unit/module/forge_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions spec/unit/module/git_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions spec/unit/module/svn_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 6 additions & 6 deletions spec/unit/module_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions spec/unit/settings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down