Merge remote-tracking branch 'origin/maint-desc-docs-fallback-1.6.x' into maint-desc-docs-fallback

Author: DavidS

lib/puppet/resource_api.rb

@@ -35,8 +35,8 @@ def register_type(definition)
     end
 
     Puppet::Type.newtype(definition[:name].to_sym) do
-      @docs = definition[:docs]
       @type_definition = type_def
+      @docs = type_def.desc
 
       # Keeps a copy of the provider around. Weird naming to avoid clashes with puppet's own `provider` member
       define_singleton_method(:my_provider) do

lib/puppet/resource_api/transport.rb

@@ -3,19 +3,16 @@ module Puppet::ResourceApi; end # rubocop:disable Style/Documentation
 # Remote target transport API
 module Puppet::ResourceApi::Transport
   def register(schema)
-    raise Puppet::DevError, 'requires a hash as schema, not `%{other_type}`' % { other_type: schema.class } unless schema.is_a? Hash
-    raise Puppet::DevError, 'requires a `:name`' unless schema.key? :name
-    raise Puppet::DevError, 'requires `:desc`' unless schema.key? :desc
-    raise Puppet::DevError, 'requires `:connection_info`' unless schema.key? :connection_info
-    raise Puppet::DevError, '`:connection_info` must be a hash, not `%{other_type}`' % { other_type: schema[:connection_info].class } unless schema[:connection_info].is_a?(Hash)
+    definition = Puppet::ResourceApi::TransportSchemaDef.new(schema)
 
-    unless transports[schema[:name]].nil?
+    unless transports[definition.name].nil?
       raise Puppet::DevError, 'Transport `%{name}` is already registered for `%{environment}`' % {
-        name: schema[:name],
+        name: definition.name,
         environment: current_environment,
       }
     end
-    transports[schema[:name]] = Puppet::ResourceApi::TransportSchemaDef.new(schema)
+
+    transports[schema[:name]] = definition
   end
   module_function :register # rubocop:disable Style/AccessModifierDeclarations
 

lib/puppet/resource_api/type_definition.rb

@@ -90,11 +90,27 @@ def namevars
       }.keys
     end
 
+    def desc
+      definition[:desc]
+    end
+
     def validate_schema(definition, attr_key)
       raise Puppet::DevError, '%{type_class} must be a Hash, not `%{other_type}`' % { type_class: self.class.name, other_type: definition.class } unless definition.is_a?(Hash)
-      @attributes = definition[attr_key]
       raise Puppet::DevError, '%{type_class} must have a name' % { type_class: self.class.name } unless definition.key? :name
-      raise Puppet::DevError, '%{type_class} must have `%{attr_key}`' % { type_class: self.class.name, attrs: attr_key } unless definition.key? attr_key
+      raise Puppet::DevError, '`%{name}` must have `%{attrs}`' % { name: definition[:name], attrs: attr_key } unless definition.key? attr_key
+
+      # 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 = definition[attr_key]
       unless attributes.is_a?(Hash)
         raise Puppet::DevError, '`%{name}.%{attrs}` must be a hash, not `%{other_type}`' % {
           name: definition[:name], attrs: attr_key, other_type: attributes.class
@@ -104,7 +120,7 @@ def validate_schema(definition, attr_key)
       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("`#{definition[:name]}.#{key}` has no documentation, add it using a `desc` key") unless attr.key? :desc
 
         # validate the type by attempting to parse into a puppet type
         @data_type_cache[attributes[key][:type]] ||=

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) { [] }
 

spec/puppet/resource_api/base_type_definition_spec.rb

@@ -4,22 +4,25 @@
   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: 'some_resource',
+      desc: 'a test type',
+      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.',
+        },
       },
-      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 }
+      features: feature_support }
   end
   let(:feature_support) { [] }
 
@@ -76,6 +79,7 @@
       let(:definition) do
         {
           name: 'some_transport',
+          desc: 'a test transport',
           connection_info: {
             username:        {
               type:      'String',
@@ -191,49 +195,80 @@
   end
 
   describe '#validate_schema' do
+    let(:definition) { { name: 'some_resource', desc: 'a test resource', attributes: attributes } }
+
     context 'when the type definition does not have a name' do
-      let(:definition) { { attributes: 'some_string' } }
+      let(:definition) { { desc: 'a test resource', attributes: {} } }
 
       it { expect { type }.to raise_error Puppet::DevError, %r{must have a name} }
     end
 
+    context 'when the type definition does not have a desc' do
+      let(:definition) { { name: 'some_resource', attributes: {} } }
+
+      it {
+        expect(Puppet).to receive(:warning).with('`some_resource` has no documentation, add it using a `desc` key')
+        type
+      }
+    end
+
     context 'when attributes is not a hash' do
-      let(:definition) { { name: 'some_resource', attributes: 'some_string' } }
+      let(:attributes) { 'some_string' }
 
       it { expect { type }.to raise_error Puppet::DevError, %r{`some_resource.attributes` must be a hash} }
     end
 
     context 'when an attribute is not a hash' do
-      let(:definition) { { name: 'some_resource', attributes: { name: 'some_string' } } }
+      let(:attributes) { { name: 'some_string' } }
 
       it { expect { type }.to raise_error Puppet::DevError, %r{`some_resource.name` must be a Hash} }
     end
 
     context 'when an attribute has no type' do
-      let(:definition) { { name: 'some_resource', attributes: { name: { desc: 'message' } } } }
+      let(:attributes) { { name: { desc: 'message' } } }
 
       it { expect { type }.to raise_error Puppet::DevError, %r{has no type} }
     end
 
-    context 'when an attribute has no descrption' do
-      let(:definition) { { name: 'some_resource', attributes: { name: { type: 'String' } } } }
+    context 'when an attribute has no description' do
+      let(:attributes) { { name: { type: 'String' } } }
 
-      it 'Raises a warning message' do
-        expect(Puppet).to receive(:warning).with('`some_resource.name` has no docs')
+      it 'raises a warning message' do
+        expect(Puppet).to receive(:warning).with('`some_resource.name` has no documentation, add it using a `desc` key')
         type
       end
     end
 
     context 'when an attribute has an unsupported type' do
-      let(:definition) { { name: 'some_resource', attributes: { name: { type: 'basic' } } } }
+      let(:definition) { { name: 'some_resource', desc: 'a test resource', attributes: { name: { type: 'basic' } } } }
 
       it { expect { type }.to raise_error %r{<basic> is not a valid type specification} }
     end
 
+    context 'with deprecated `docs` key' do
+      let(:definition) { { name: 'some_resource', docs: 'a test resource', attributes: {} } }
+
+      it 'does not raise a warning message' do
+        expect(Puppet).not_to receive(:warning)
+        type
+      end
+
+      it 'exposes the documentation as `desc`' do
+        expect(type.desc).to eq 'a test resource'
+      end
+    end
+
+    context 'with both `docs` and `desc` key' do
+      let(:definition) { { name: 'some_resource', desc: 'the real description', docs: 'an oversight', attributes: {} } }
+
+      it { expect { type }.to raise_error Puppet::DevError, %r{`some_resource` has both `desc` and `docs`, prefer using `desc`} }
+    end
+
     context 'with both behavior and behaviour' do
       let(:definition) do
         {
           name: 'bad_behaviour',
+          desc: 'a test type',
           attributes: {
             name: {
               type: 'String',
@@ -251,6 +286,7 @@
       let(:definition) do
         {
           name: 'bad_syntax',
+          desc: 'a test type',
           attributes: {
             name: {
               type: 'Optional[String',

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

spec/puppet/resource_api/transport_spec.rb

@@ -36,11 +36,18 @@ def change_environment(name = nil)
 
   describe '#register(schema)' do
     context 'when registering a schema with missing keys' do
-      it { expect { described_class.register([]) }.to raise_error(Puppet::DevError, %r{requires a hash as schema}) }
-      it { expect { described_class.register({}) }.to raise_error(Puppet::DevError, %r{requires a `:name`}) }
-      it { expect { described_class.register(name: 'no connection info', desc: 'some description') }.to raise_error(Puppet::DevError, %r{requires `:connection_info`}) }
-      it { expect { described_class.register(name: 'no description') }.to raise_error(Puppet::DevError, %r{requires `:desc`}) }
-      it { expect { described_class.register(name: 'no hash attributes', desc: 'some description', connection_info: []) }.to raise_error(Puppet::DevError, %r{`:connection_info` must be a hash, not}) }
+      it { expect { described_class.register([]) }.to raise_error(Puppet::DevError, %r{must be a Hash, not `Array`}) }
+      it { expect { described_class.register({}) }.to raise_error(Puppet::DevError, %r{must have a name}) }
+      it { expect { described_class.register(name: 'no connection info', desc: 'some description') }.to raise_error(Puppet::DevError, %r{`no connection info` must have `connection_info`}) }
+      it do
+        expect {
+          described_class.register(name: 'no hash attributes', desc: 'some description', connection_info: [])
+        }.to raise_error(Puppet::DevError, %r{`no hash attributes.connection_info` must be a hash, not `Array})
+      end
+      it 'warns if there is no documentation ' do
+        expect(Puppet).to receive(:warning).with(%r{has no documentation, add it using a `desc` key})
+        described_class.register(name: 'no description', connection_info: {})
+      end
     end
 
     context 'when registering a minimal transport' do
@@ -300,6 +307,7 @@ class Wibble; end
 
       it 'validates the connection_info' do
         allow(Puppet::ResourceApi::TransportSchemaDef).to receive(:new).with(schema).and_return(schema_def)
+        allow(schema_def).to receive(:name).with(no_args).and_return('validate')
 
         described_class.register(schema)
 
@@ -330,6 +338,7 @@ class Wibble; end
 
     before(:each) do
       allow(Puppet::ResourceApi::TransportSchemaDef).to receive(:new).with(schema).and_return(schema_def)
+      allow(schema_def).to receive(:name).with(no_args).and_return('sensitive_transport')
       described_class.register(schema)
     end
 

spec/puppet/resource_api_spec.rb

@@ -16,6 +16,7 @@
 
   after(:each) do
     Puppet::Util::Log.close_all
+    Puppet::Util::NetworkDevice.teardown
   end
 
   it 'has a version number' do
@@ -44,6 +45,7 @@
     let(:definition) do
       {
         name: type_name,
+        desc: 'a test resource',
         attributes: {
           name: {
             type: 'String',
@@ -747,6 +749,7 @@ def set(_context, _changes); end
     let(:definition) do
       {
         name: 'init_behaviour',
+        desc: 'a test resource',
         attributes: {
           ensure: {
             type: 'Enum[present, absent]',
@@ -1172,7 +1175,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
@@ -1792,6 +1795,7 @@ def set(_context, changes) end
     let(:definition) do
       {
         name: 'test_noop_support',
+        desc: 'a test resource',
         features: ['no such feature'],
         attributes: {},
       }
@@ -1808,6 +1812,7 @@ def set(_context, changes) end
       let(:definition) do
         {
           name: 'test_behaviour',
+          desc: 'a test resource',
           attributes: {
             id: {
               type: 'String',
@@ -1824,6 +1829,7 @@ def set(_context, changes) end
       let(:definition) do
         {
           name: 'test_behaviour',
+          desc: 'a test resource',
           attributes: {
             param: {
               type: 'String',
@@ -1840,6 +1846,7 @@ def set(_context, changes) end
       let(:definition) do
         {
           name: 'test_behaviour',
+          desc: 'a test resource',
           attributes: {
             param_ro: {
               type: 'String',
@@ -1856,6 +1863,7 @@ def set(_context, changes) end
       let(:definition) do
         {
           name: 'test_behaviour',
+          desc: 'a test resource',
           attributes: {
             param_ro: {
               type: 'String',
@@ -1872,6 +1880,7 @@ def set(_context, changes) end
       let(:definition) do
         {
           name: 'test_behaviour',
+          desc: 'a test resource',
           attributes: {
             source: {
               type: 'String',