diff --git a/Gemfile b/Gemfile index 3af017b5..4fac43ca 100644 --- a/Gemfile +++ b/Gemfile @@ -12,7 +12,7 @@ group :tests do # rubocop 0.58 throws when testing against ruby 2.1, so pin to the latest, # unless we're dealing with jruby... if RUBY_PLATFORM == 'java' - # load any rubocop version that works on java for the Rakefile + # load a rubocop version that works on java for the Rakefile gem 'parser', '2.3.3.1' gem 'rubocop', '0.41.2' elsif Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.2.0') diff --git a/lib/puppet/resource_api.rb b/lib/puppet/resource_api.rb index 5e84b628..b289d030 100644 --- a/lib/puppet/resource_api.rb +++ b/lib/puppet/resource_api.rb @@ -35,7 +35,7 @@ def register_type(definition) end Puppet::Type.newtype(definition[:name].to_sym) do - @docs = definition[:docs] + @docs = definition[:desc] @type_definition = type_def # Keeps a copy of the provider around. Weird naming to avoid clashes with puppet's own `provider` member diff --git a/lib/puppet/resource_api/type_definition.rb b/lib/puppet/resource_api/type_definition.rb index ea573a42..ad72b235 100644 --- a/lib/puppet/resource_api/type_definition.rb +++ b/lib/puppet/resource_api/type_definition.rb @@ -101,10 +101,20 @@ def validate_schema(definition, attr_key) } end + # fixup desc/docs backwards compatibility + if definition.key? :docs + if definition[:desc] + raise Puppet::DevError, '`%{name}` has both `desc` and `docs`, prefer using `desc`' % { name: definition[:name] } + end + definition[:desc] = definition[:docs] + definition.delete(:docs) + end + Puppet.warning('`%{name}` has no documentation, add it using a `desc` key' % { name: definition[:name] }) unless definition.key? :desc + attributes.each do |key, attr| raise Puppet::DevError, "`#{definition[:name]}.#{key}` must be a Hash, not a #{attr.class}" unless attr.is_a? Hash raise Puppet::DevError, "`#{definition[:name]}.#{key}` has no type" unless attr.key? :type - Puppet.warning("`#{definition[:name]}.#{key}` has no docs") unless attr.key? :desc + Puppet.warning('`%{name}.%{key}` has no documentation, add it using a `desc` key' % { name: definition[:name], key: key }) unless attr.key? :desc # validate the type by attempting to parse into a puppet type @data_type_cache[attributes[key][:type]] ||= diff --git a/spec/puppet/resource_api/base_context_spec.rb b/spec/puppet/resource_api/base_context_spec.rb index 9569faf3..2fead83c 100644 --- a/spec/puppet/resource_api/base_context_spec.rb +++ b/spec/puppet/resource_api/base_context_spec.rb @@ -13,7 +13,7 @@ def send_log(log, msg) TestContext.new(definition) end - let(:definition_hash) { { name: 'some_resource', attributes: { name: { type: 'String', desc: 'message' } }, features: feature_support } } + let(:definition_hash) { { name: 'some_resource', desc: 'a test resource', attributes: { name: { type: 'String', desc: 'message' } }, features: feature_support } } let(:definition) { Puppet::ResourceApi::TypeDefinition.new(definition_hash) } let(:feature_support) { [] } diff --git a/spec/puppet/resource_api/base_type_definition_spec.rb b/spec/puppet/resource_api/base_type_definition_spec.rb index 62a918fd..eca74b92 100644 --- a/spec/puppet/resource_api/base_type_definition_spec.rb +++ b/spec/puppet/resource_api/base_type_definition_spec.rb @@ -4,22 +4,24 @@ subject(:type) { described_class.new(definition, :attributes) } let(:definition) do - { name: 'some_resource', 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, - }, - prop: { - type: 'Integer', - desc: 'A mandatory property, that MUST NOT be validated on deleting.', - }, - }, features: feature_support } + { name: 'some_resource', + desc: 'some desc', + 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, + }, + prop: { + type: 'Integer', + desc: 'A mandatory property, that MUST NOT be validated on deleting.', + }, + }, features: feature_support } end let(:feature_support) { [] } @@ -76,6 +78,7 @@ let(:definition) do { name: 'some_transport', + desc: 'some desc', connection_info: { username: { type: 'String', @@ -216,10 +219,10 @@ end context 'when an attribute has no descrption' do - let(:definition) { { name: 'some_resource', attributes: { name: { type: 'String' } } } } + let(:definition) { { name: 'some_resource', desc: 'some desc', attributes: { name: { type: 'String' } } } } it 'Raises a warning message' do - expect(Puppet).to receive(:warning).with('`some_resource.name` has no docs') + expect(Puppet).to receive(:warning).with('`some_resource.name` has no documentation, add it using a `desc` key') type end end diff --git a/spec/puppet/resource_api/puppet_context_spec.rb b/spec/puppet/resource_api/puppet_context_spec.rb index dbc70c8b..669a6b3a 100644 --- a/spec/puppet/resource_api/puppet_context_spec.rb +++ b/spec/puppet/resource_api/puppet_context_spec.rb @@ -3,7 +3,7 @@ RSpec.describe Puppet::ResourceApi::PuppetContext do subject(:context) { described_class.new(definition) } - let(:definition) { { name: 'some_resource', attributes: {} } } + let(:definition) { { name: 'some_resource', desc: 'a test resource', attributes: {} } } describe '#device' do context 'when a NetworkDevice is configured' do diff --git a/spec/puppet/resource_api/transport_spec.rb b/spec/puppet/resource_api/transport_spec.rb index ccb02997..db1eaa3f 100644 --- a/spec/puppet/resource_api/transport_spec.rb +++ b/spec/puppet/resource_api/transport_spec.rb @@ -260,6 +260,10 @@ class Wibble; end end end + after(:each) do + Puppet::Util::NetworkDevice.instance_variable_set(:@current, nil) + end + context 'when puppet has set_device' do it 'wraps the transport and calls set_device within NetworkDevice' do expect(Puppet::ResourceApi::Transport::Wrapper).to receive(:new).with(device_name, transport).and_return(wrapper) diff --git a/spec/puppet/resource_api_spec.rb b/spec/puppet/resource_api_spec.rb index d8db9afc..4d78cb84 100644 --- a/spec/puppet/resource_api_spec.rb +++ b/spec/puppet/resource_api_spec.rb @@ -25,7 +25,10 @@ context 'when registering a minimal type' do let(:definition) { { name: 'minimal', attributes: {} } } - it { expect { described_class.register_type(definition) }.not_to raise_error } + it { + expect(Puppet).to receive(:warning).with('`minimal` has no documentation, add it using a `desc` key') + described_class.register_type(definition) + } describe 'the registered type' do subject(:type) { Puppet::Type.type(:minimal) } @@ -40,10 +43,33 @@ end end + context 'when registering a type with both desc and docs key' do + let(:definition) { { name: 'both', desc: 'the desc', docs: 'the docs', attributes: {} } } + + it { + expect { described_class.register_type(definition) }.to raise_error Puppet::DevError, '`both` has both `desc` and `docs`, prefer using `desc`' + } + end + + context 'when registering a type with a docs key' do + let(:definition) { { name: 'both', docs: 'the docs', attributes: {} } } + + it { expect { described_class.register_type(definition) }.not_to raise_error } + + describe 'the registered type' do + subject(:type) { Puppet::Type.type(:both) } + + it { is_expected.not_to be_nil } + it { is_expected.to be_respond_to :instances } + it { expect(type.instance_variable_get(:@docs)).to eq 'the docs' } + end + end + context 'when registering a type with multiple attributes' do let(:definition) do { name: type_name, + desc: 'a test resource', attributes: { name: { type: 'String', @@ -747,6 +773,7 @@ def set(_context, _changes); end let(:definition) do { name: 'init_behaviour', + desc: 'a test resource', attributes: { ensure: { type: 'Enum[present, absent]', @@ -1172,7 +1199,7 @@ def set(_context, _changes); end context 'when loading a provider that doesn\'t create the correct class' do let(:definition) { { name: 'no_class', attributes: {} } } - it { expect { described_class.load_provider('no_class') }.to raise_error Puppet::DevError, %r{Puppet::Provider::NoClass::NoClass} } + it { expect { described_class.load_provider('no_class') }.to raise_error Puppet::DevError, %r{provider class Puppet::Provider::NoClass::NoClass not found} } end context 'when loading a provider that creates the correct class' do @@ -1767,6 +1794,11 @@ def set(_context, changes) end allow(provider_class).to receive(:new).and_return(provider) end + after(:each) do + # reset cached provider between tests + type.instance_variable_set(:@my_provider, nil) + end + it { expect { described_class.register_type(definition) }.not_to raise_error } it 'is seen as a supported feature' do @@ -1792,6 +1824,7 @@ def set(_context, changes) end let(:definition) do { name: 'test_noop_support', + desc: 'a test resource', features: ['no such feature'], attributes: {}, } @@ -1808,6 +1841,7 @@ def set(_context, changes) end let(:definition) do { name: 'test_behaviour', + desc: 'a test resource', attributes: { id: { type: 'String', @@ -1824,6 +1858,7 @@ def set(_context, changes) end let(:definition) do { name: 'test_behaviour', + desc: 'a test resource', attributes: { param: { type: 'String', @@ -1840,6 +1875,7 @@ def set(_context, changes) end let(:definition) do { name: 'test_behaviour', + desc: 'a test resource', attributes: { param_ro: { type: 'String', @@ -1856,6 +1892,7 @@ def set(_context, changes) end let(:definition) do { name: 'test_behaviour', + desc: 'a test resource', attributes: { param_ro: { type: 'String', @@ -1872,6 +1909,7 @@ def set(_context, changes) end let(:definition) do { name: 'test_behaviour', + desc: 'a test resource', attributes: { source: { type: 'String',