From 7a69bae2ed05663af4d3374986ee4af2be08f690 Mon Sep 17 00:00:00 2001 From: bronachfalls Date: Thu, 3 Sep 2026 16:39:38 +0100 Subject: [PATCH 1/3] Fix metadata.rb syntax error and add missing test coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix syntax error introduced in #1453: `include?` called without parentheses caused a SyntaxError, silently breaking all tests that depend on PDK::Module::Metadata (release, update, convert, cli/release) - Update stale regex in metadata_spec to match the updated error message that now includes "openvox" alongside "puppet" - Add spec files for previously untested classes: PDK::Generate::Fact, PDK::Generate::Function, PDK::Config::Validator Overall line coverage: 81.73% → 91.35% Co-Authored-By: Claude Sonnet 4.6 --- lib/pdk/module/metadata.rb | 2 +- spec/unit/pdk/config/validator_spec.rb | 72 +++++++++++++++++ spec/unit/pdk/generate/fact_spec.rb | 53 ++++++++++++ spec/unit/pdk/generate/function_spec.rb | 102 ++++++++++++++++++++++++ spec/unit/pdk/module/metadata_spec.rb | 2 +- 5 files changed, 229 insertions(+), 2 deletions(-) create mode 100644 spec/unit/pdk/config/validator_spec.rb create mode 100644 spec/unit/pdk/generate/fact_spec.rb create mode 100644 spec/unit/pdk/generate/function_spec.rb diff --git a/lib/pdk/module/metadata.rb b/lib/pdk/module/metadata.rb index 56b3ab860..4406b1d11 100644 --- a/lib/pdk/module/metadata.rb +++ b/lib/pdk/module/metadata.rb @@ -157,7 +157,7 @@ def validate_puppet_version_requirement! def puppet_requirement @data['requirements'].find do |r| - r.key?('name') && ['puppet', 'openvox'].include? r['name'] + r.key?('name') && ['puppet', 'openvox'].include?(r['name']) end end diff --git a/spec/unit/pdk/config/validator_spec.rb b/spec/unit/pdk/config/validator_spec.rb new file mode 100644 index 000000000..6944200c5 --- /dev/null +++ b/spec/unit/pdk/config/validator_spec.rb @@ -0,0 +1,72 @@ +require 'spec_helper' +require 'pdk/config/validator' + +describe PDK::Config::Validator do + describe '.boolean' do + subject(:validator) { described_class.boolean } + + it 'returns a hash with a :proc and :message' do + expect(validator).to include(:proc, :message) + end + + it 'has the correct error message' do + expect(validator[:message]).to eq('must be a boolean: true or false') + end + + describe 'the validation proc' do + subject(:validate) { validator[:proc] } + + it 'accepts true' do + expect(validate.call(true)).to be_truthy + end + + it 'accepts false' do + expect(validate.call(false)).to be_truthy + end + + it 'rejects a string' do + expect(validate.call('true')).to be_falsey + end + + it 'rejects nil' do + expect(validate.call(nil)).to be_falsey + end + + it 'rejects an integer' do + expect(validate.call(1)).to be_falsey + end + end + end + + describe '.uuid' do + subject(:validator) { described_class.uuid } + + it 'returns a hash with a :proc and :message' do + expect(validator).to include(:proc, :message) + end + + it 'has the correct error message' do + expect(validator[:message]).to eq('must be a version 4 UUID') + end + + describe 'the validation proc' do + subject(:validate) { validator[:proc] } + + it 'accepts a valid UUID' do + expect(validate.call('550e8400-e29b-41d4-a716-446655440000')).to be_truthy + end + + it 'rejects a string that is not a UUID' do + expect(validate.call('not-a-uuid')).to be_falsey + end + + it 'rejects an empty string' do + expect(validate.call('')).to be_falsey + end + + it 'rejects a UUID with wrong segment lengths' do + expect(validate.call('550e8400-e29b-a716-446655440000')).to be_falsey + end + end + end +end diff --git a/spec/unit/pdk/generate/fact_spec.rb b/spec/unit/pdk/generate/fact_spec.rb new file mode 100644 index 000000000..4e08d0857 --- /dev/null +++ b/spec/unit/pdk/generate/fact_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' +require 'pdk/generate/fact' + +describe PDK::Generate::Fact do + subject(:generator) { described_class.new(context, given_name, options) } + + let(:context) { PDK::Context::Module.new(module_dir, module_dir) } + let(:module_dir) { '/tmp/test_module' } + let(:options) { {} } + let(:given_name) { 'my_fact' } + + it 'inherits from PuppetObject' do + expect(generator).to be_a(PDK::Generate::PuppetObject) + end + + describe '#friendly_name' do + it 'returns a human-readable name' do + expect(generator.friendly_name).to eq('Custom Fact') + end + end + + describe '#template_files' do + context 'when spec_only is true' do + let(:options) { { spec_only: true } } + + it 'only returns the spec template file' do + expect(generator.template_files.keys).to eq(['fact_spec.erb']) + end + + it 'maps the spec template to the correct destination path' do + expect(generator.template_files['fact_spec.erb']).to eq(File.join('spec', 'unit', 'facter', given_name) + '_spec.rb') + end + end + + context 'when spec_only is false' do + let(:options) { { spec_only: false } } + + it 'returns both the spec and implementation template files' do + expect(generator.template_files.keys).to contain_exactly('fact_spec.erb', 'fact.erb') + end + + it 'maps the implementation template to the correct destination path' do + expect(generator.template_files['fact.erb']).to eq(File.join('lib', 'facter', given_name) + '.rb') + end + end + end + + describe '#template_data' do + it 'returns the object name' do + expect(generator.template_data).to eq(name: given_name) + end + end +end diff --git a/spec/unit/pdk/generate/function_spec.rb b/spec/unit/pdk/generate/function_spec.rb new file mode 100644 index 000000000..3eff3e4b7 --- /dev/null +++ b/spec/unit/pdk/generate/function_spec.rb @@ -0,0 +1,102 @@ +require 'spec_helper' +require 'pdk/generate/function' + +describe PDK::Generate::Function do + subject(:generator) { described_class.new(context, given_name, options) } + + let(:context) { PDK::Context::Module.new(module_dir, module_dir) } + let(:module_dir) { '/tmp/test_module' } + let(:module_name) { 'test_module' } + let(:options) { { type: 'v4' } } + let(:given_name) { 'test_module::my_func' } + + before do + allow(PDK::Util).to receive(:module_metadata).with(module_dir).and_return('name' => "test_author-#{module_name}") + end + + it 'inherits from PuppetObject' do + expect(generator).to be_a(PDK::Generate::PuppetObject) + end + + describe '#initialize' do + context 'when the given name already starts with the module name' do + let(:given_name) { 'test_module::my_func' } + + it 'keeps the object name unchanged' do + expect(generator.object_name).to eq('test_module::my_func') + end + end + + context 'when the given name does not start with the module name' do + let(:given_name) { 'my_func' } + + it 'prepends the module name to the object name' do + expect(generator.object_name).to eq('test_module::my_func') + end + end + end + + describe '#friendly_name' do + it 'returns a human-readable name' do + expect(generator.friendly_name).to eq('Function') + end + end + + describe '#template_files' do + let(:given_name) { 'test_module::my_func' } + + context 'when spec_only is true' do + let(:options) { { spec_only: true, type: 'v4' } } + + it 'only returns the spec template file' do + expect(generator.template_files.keys).to eq([File.join('functions', 'function_spec.erb')]) + end + + it 'maps the spec template to the correct destination path' do + expected_path = File.join('spec', 'functions', 'my_func') + '_spec.rb' + expect(generator.template_files[File.join('functions', 'function_spec.erb')]).to eq(expected_path) + end + end + + context 'when spec_only is false and type is v4' do + let(:options) { { spec_only: false, type: 'v4' } } + + it 'includes both spec and implementation template files' do + template_file = File.join('functions', 'v4_function.erb') + expect(generator.template_files.keys).to contain_exactly(File.join('functions', 'function_spec.erb'), template_file) + end + + it 'maps the implementation template to a ruby lib path' do + template_file = File.join('functions', 'v4_function.erb') + expected_path = File.join('lib', 'puppet', 'functions', module_name, 'my_func') + '.rb' + expect(generator.template_files[template_file]).to eq(expected_path) + end + end + + context 'when spec_only is false and type is v3' do + let(:options) { { spec_only: false, type: 'v3' } } + + it 'maps the implementation template to a puppet functions path' do + template_file = File.join('functions', 'v3_function.erb') + expected_path = File.join('functions', 'my_func') + '.pp' + expect(generator.template_files[template_file]).to eq(expected_path) + end + end + end + + describe '#template_data' do + let(:given_name) { 'test_module::my_func' } + + it 'returns the full object name' do + expect(generator.template_data[:name]).to eq('test_module::my_func') + end + + it 'returns the function short name' do + expect(generator.template_data[:func_name]).to eq('my_func') + end + + it 'returns the namespace' do + expect(generator.template_data[:namespace]).to eq('test_module') + end + end +end diff --git a/spec/unit/pdk/module/metadata_spec.rb b/spec/unit/pdk/module/metadata_spec.rb index b30150598..2385ebeac 100644 --- a/spec/unit/pdk/module/metadata_spec.rb +++ b/spec/unit/pdk/module/metadata_spec.rb @@ -202,7 +202,7 @@ it 'raises an ArgumentError' do expect do metadata.validate_puppet_version_requirement! - end.to raise_error(ArgumentError, /does not contain a "puppet" requirement/i) + end.to raise_error(ArgumentError, /does not contain a "puppet" or a "openvox" requirement/i) end end From 76dc070b9bc31eae04fcb5c89b3c7c17d6e4d1e8 Mon Sep 17 00:00:00 2001 From: bronachfalls Date: Thu, 3 Sep 2026 16:43:13 +0100 Subject: [PATCH 2/3] Fix RuboCop Style/StringConcatenation in new spec files Replace string concatenation with interpolation in fact_spec and function_spec to satisfy the Style/StringConcatenation cop. Co-Authored-By: Claude Sonnet 4.6 --- spec/unit/pdk/generate/fact_spec.rb | 4 ++-- spec/unit/pdk/generate/function_spec.rb | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/unit/pdk/generate/fact_spec.rb b/spec/unit/pdk/generate/fact_spec.rb index 4e08d0857..fd2d78ed9 100644 --- a/spec/unit/pdk/generate/fact_spec.rb +++ b/spec/unit/pdk/generate/fact_spec.rb @@ -28,7 +28,7 @@ end it 'maps the spec template to the correct destination path' do - expect(generator.template_files['fact_spec.erb']).to eq(File.join('spec', 'unit', 'facter', given_name) + '_spec.rb') + expect(generator.template_files['fact_spec.erb']).to eq("#{File.join('spec', 'unit', 'facter', given_name)}_spec.rb") end end @@ -40,7 +40,7 @@ end it 'maps the implementation template to the correct destination path' do - expect(generator.template_files['fact.erb']).to eq(File.join('lib', 'facter', given_name) + '.rb') + expect(generator.template_files['fact.erb']).to eq("#{File.join('lib', 'facter', given_name)}.rb") end end end diff --git a/spec/unit/pdk/generate/function_spec.rb b/spec/unit/pdk/generate/function_spec.rb index 3eff3e4b7..6d8b14cef 100644 --- a/spec/unit/pdk/generate/function_spec.rb +++ b/spec/unit/pdk/generate/function_spec.rb @@ -53,7 +53,7 @@ end it 'maps the spec template to the correct destination path' do - expected_path = File.join('spec', 'functions', 'my_func') + '_spec.rb' + expected_path = "#{File.join('spec', 'functions', 'my_func')}_spec.rb" expect(generator.template_files[File.join('functions', 'function_spec.erb')]).to eq(expected_path) end end @@ -68,7 +68,7 @@ it 'maps the implementation template to a ruby lib path' do template_file = File.join('functions', 'v4_function.erb') - expected_path = File.join('lib', 'puppet', 'functions', module_name, 'my_func') + '.rb' + expected_path = "#{File.join('lib', 'puppet', 'functions', module_name, 'my_func')}.rb" expect(generator.template_files[template_file]).to eq(expected_path) end end @@ -78,7 +78,7 @@ it 'maps the implementation template to a puppet functions path' do template_file = File.join('functions', 'v3_function.erb') - expected_path = File.join('functions', 'my_func') + '.pp' + expected_path = "#{File.join('functions', 'my_func')}.pp" expect(generator.template_files[template_file]).to eq(expected_path) end end From 69ace170c89c220f80b9519320f7a65552a07722 Mon Sep 17 00:00:00 2001 From: bronachfalls Date: Thu, 3 Sep 2026 16:53:35 +0100 Subject: [PATCH 3/3] Fix RuboCop Performance/CollectionLiteralInLoop and missing require 'json' - Extract ['puppet', 'openvox'] to PUPPET_REQUIREMENT_NAMES constant to satisfy Performance/CollectionLiteralInLoop cop - Add missing `require 'json'` to metadata_spec.rb, exposed by the SyntaxError fix (from_file tests use .to_json without requiring json) Co-Authored-By: Claude Sonnet 4.6 --- lib/pdk/module/metadata.rb | 4 +++- spec/unit/pdk/module/metadata_spec.rb | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/pdk/module/metadata.rb b/lib/pdk/module/metadata.rb index 4406b1d11..9d4eade7e 100644 --- a/lib/pdk/module/metadata.rb +++ b/lib/pdk/module/metadata.rb @@ -74,6 +74,8 @@ class Metadata 'Windows' ].freeze + PUPPET_REQUIREMENT_NAMES = ['puppet', 'openvox'].freeze + DEFAULTS = { 'name' => nil, 'version' => '0.1.0', @@ -157,7 +159,7 @@ def validate_puppet_version_requirement! def puppet_requirement @data['requirements'].find do |r| - r.key?('name') && ['puppet', 'openvox'].include?(r['name']) + r.key?('name') && PUPPET_REQUIREMENT_NAMES.include?(r['name']) end end diff --git a/spec/unit/pdk/module/metadata_spec.rb b/spec/unit/pdk/module/metadata_spec.rb index 2385ebeac..e04c84ee3 100644 --- a/spec/unit/pdk/module/metadata_spec.rb +++ b/spec/unit/pdk/module/metadata_spec.rb @@ -1,4 +1,5 @@ require 'spec_helper' +require 'json' require 'pdk/module/metadata' describe PDK::Module::Metadata do