On Oct 22, 2011, at 12:56 PM, Stefan Schulte wrote: > Hi Luke, > > I'm currently reviewing the patchset. One general question: Is this > targeted to a specific release or ready to merge?
It was not targeted to a specific release - just something that I thought should go into master. > From what I've read so far it should not break custom types/providers > but the changes are important to know if you are writing tests for > your custom types/providers (like the removal of the attrclass > classmethod) Yeah, the changes were definitely important. It is worth considering adding back compatibility methods with warnings - I think there's only one method that I keep but change the behavior of, although I think it's the 'parameters' method and thus a big one. > 2 specific comments inline. I don't think this is a pull request; should I add one for this code? > On Tue, Jul 05, 2011 at 03:40:53PM -0700, Luke Kanies wrote: >> The previous collection of methods for creating and >> accessing parameters was very messy, with different >> methods for properties, parameters, and metaparams, >> and this trims it all into a few (mostly) simple methods, >> and should allow us to deprecate many methods. >> >> Signed-off-by: Luke Kanies <[email protected]> >> --- >> Local-branch: refactor/master/8233-refactor_parameter_management >> lib/puppet/parameter.rb | 18 ++ >> lib/puppet/provider.rb | 2 +- >> lib/puppet/provider/aixobject.rb | 2 +- >> lib/puppet/provider/parsedfile.rb | 3 +- >> lib/puppet/reference/metaparameter.rb | 10 +- >> lib/puppet/reference/type.rb | 7 +- >> lib/puppet/type.rb | 309 ++++++++++++++++++-------------- >> spec/unit/parameter_spec.rb | 21 +++ >> spec/unit/type_spec.rb | 153 +++++++++++++++-- >> test/ral/manager/attributes.rb | 40 +---- >> test/ral/manager/type.rb | 6 +- >> test/ral/providers/provider.rb | 4 +- >> test/ral/type/exec.rb | 1 + >> test/ral/type/file/target.rb | 2 +- >> test/ral/type/resources.rb | 10 +- >> 15 files changed, 381 insertions(+), 207 deletions(-) >> >> diff --git a/lib/puppet/parameter.rb b/lib/puppet/parameter.rb >> index 29d60fc..da9c00b 100644 >> --- a/lib/puppet/parameter.rb >> +++ b/lib/puppet/parameter.rb >> @@ -123,6 +123,24 @@ class Puppet::Parameter >> end >> end >> >> + def self.property? >> + ancestors.include?(Puppet::Property) >> + end >> + >> + def self.metaparameter? >> + Puppet::Type.metaparam?(name) >> + end >> + >> + def self.parameter? >> + ! property? and ! metaparameter? >> + end >> + >> + def self.parameter_type >> + property? and return :property >> + metaparameter? and return :metaparameter >> + return :parameter >> + end >> + >> # Just a simple method to proxy instance methods to class methods >> def self.proxymethods(*values) >> values.each { |val| >> diff --git a/lib/puppet/provider.rb b/lib/puppet/provider.rb >> index 4456feb..e95613b 100644 >> --- a/lib/puppet/provider.rb >> +++ b/lib/puppet/provider.rb >> @@ -143,7 +143,7 @@ class Puppet::Provider >> # They all get stored in @property_hash. This method is useful >> # for those providers that use prefetch and flush. >> def self.mk_resource_methods >> - [resource_type.validproperties, resource_type.parameters].flatten.each >> do |attr| >> + resource_type.parameter_names.each do |attr| >> attr = symbolize(attr) >> next if attr == :name >> define_method(attr) do >> diff --git a/lib/puppet/provider/aixobject.rb >> b/lib/puppet/provider/aixobject.rb >> index 9506c67..3ab6a9f 100755 >> --- a/lib/puppet/provider/aixobject.rb >> +++ b/lib/puppet/provider/aixobject.rb >> @@ -341,7 +341,7 @@ class Puppet::Provider::AixObject < Puppet::Provider >> # It creates getter/setter methods for each property our resource type >> supports. >> # If setter or getter already defined it will not be overwritten >> def self.mk_resource_methods >> - [resource_type.validproperties, resource_type.parameters].flatten.each >> do |prop| >> + resource_type.parameter_names.each do |prop| >> next if prop == :ensure >> define_method(prop) { get(prop) || :absent} unless >> public_method_defined?(prop) >> define_method(prop.to_s + "=") { |*vals| set(prop, *vals) } unless >> public_method_defined?(prop.to_s + "=") >> diff --git a/lib/puppet/provider/parsedfile.rb >> b/lib/puppet/provider/parsedfile.rb >> index 75a215f..009ad1f 100755 >> --- a/lib/puppet/provider/parsedfile.rb >> +++ b/lib/puppet/provider/parsedfile.rb >> @@ -126,8 +126,7 @@ class Puppet::Provider::ParsedFile < Puppet::Provider >> >> # Override the default method with a lot more functionality. >> def self.mk_resource_methods >> - [resource_type.validproperties, resource_type.parameters].flatten.each >> do |attr| >> - attr = symbolize(attr) >> + resource_type.parameter_names.each do |attr| >> define_method(attr) do >> # if @property_hash.empty? >> # # Note that this swaps the provider out from under us. >> diff --git a/lib/puppet/reference/metaparameter.rb >> b/lib/puppet/reference/metaparameter.rb >> index 3c4c087..d19e62c 100644 >> --- a/lib/puppet/reference/metaparameter.rb >> +++ b/lib/puppet/reference/metaparameter.rb >> @@ -22,14 +22,10 @@ in your manifest, including defined components. >> } >> begin >> params = [] >> - Puppet::Type.eachmetaparam { |param| >> - params << param >> - } >> - >> - params.sort { |a,b| >> - a.to_s <=> b.to_s >> + Puppet::Type.metaparameters.sort { |a,b| >> + a.name.to_s <=> b.name.to_s >> }.each { |param| >> - str += paramwrap(param.to_s, scrub(Puppet::Type.metaparamdoc(param)), >> :level => 3) >> + str += paramwrap(param.to_s, scrub(param.doc), :level => 3) >> } >> rescue => detail >> puts detail.backtrace >> diff --git a/lib/puppet/reference/type.rb b/lib/puppet/reference/type.rb >> index b423387..a4a8d98 100644 >> --- a/lib/puppet/reference/type.rb >> +++ b/lib/puppet/reference/type.rb >> @@ -94,10 +94,9 @@ type = Puppet::Util::Reference.newreference :type, :doc >> => "All Puppet resource >> >> str += h("Parameters", 4) + "\n" >> type.parameters.sort { |a,b| >> - a.to_s <=> b.to_s >> - }.each { |name,param| >> - #docs[name] = indent(scrub(type.paramdoc(name)), $tab) >> - docs[name] = scrub(type.paramdoc(name)) >> + a.name.to_s <=> b.name.to_s >> + }.each { |param| >> + docs[param.name] = scrub(param.doc) >> } >> >> additional_key_attributes = type.key_attributes - [:name] >> diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb >> index 29a0e58..77195a2 100644 >> --- a/lib/puppet/type.rb >> +++ b/lib/puppet/type.rb >> @@ -161,56 +161,179 @@ class Type >> @typeloader >> end >> >> - ############################### >> - # Code related to resource type attributes. >> class << self >> include Puppet::Util::ClassGen >> include Puppet::Util::Warnings >> - attr_reader :properties >> end >> >> + ############################### >> + # New paramcode to define the new interface >> + >> + # A method for adding a new parameter to the list of known parameters. >> + # These variables should never be accessed directly anywhere other than >> the >> + # gate methods defined here. >> + def self.add_parameter_class(klass) >> + @parameters ||= [] >> + >> + @parameters << klass >> + @parameters.sort! { |a,b| >> + case >> + when a.isnamevar? || (a.name == :name) >> + -1 >> + when b.isnamevar? || (b.name == :name) >> + 1 >> + when a.name == :provider >> + -1 >> + when b.name == :provider >> + 1 >> + when a.name == :ensure >> + -1 >> + when b.name == :ensure >> + 1 >> + when a.parameter? && b.property? >> + -1 >> + when a.property? && b.parameter? >> + 1 >> + else >> + 0 >> + end >> + } >> + @parameters_by_name ||= {} >> + @parameters_by_name[klass.name] = klass >> + end >> + >> + # Return the list of all parameter classes supported by this class. >> + # Always includes metaparameters. >> + def self.parameters >> + @parameters ||= [] >> + >> + # We're doing something slightly magical here - if a parameter >> + # are defined in Puppet::Type (rather than a subclass), then it's >> + # a metaparam. Thus, we have to behave a bit differently if we're >> + # the base class. >> + if self == Puppet::Type >> + @parameters.dup >> + else >> + @parameters + metaparameters >> + end >> + end >> + >> + def self.parameter_names >> + parameters.collect { |p| p.name } >> + end >> + >> + # This returns the class for a given parameter, or nil. >> + def self.parameter(name) >> + name = name.to_sym >> + @parameters_by_name ||= {} >> + if self == Puppet::Type >> + @parameters_by_name[name] >> + else >> + @parameters_by_name[name] || Puppet::Type.parameter(name) >> + end >> + end >> + >> + # What kind of parameter do we have - property, parameter, or >> metaparameter? >> + def self.parameter_type(name) >> + name = name.to_sym >> + return nil unless klass = parameter(name) >> + klass.parameter_type >> + end >> + >> + # Is the provided name a valid parameter? >> + # This method is used in both Puppet::Type and Puppet::Resource. >> + def self.valid_parameter?(name) >> + name = name.to_sym >> + return true if name == :name >> + return true if parameter(name) >> + return false >> + end >> + >> + # Is the parameter in question a meta-parameter? >> + def self.metaparam?(param) >> + Puppet::Type.valid_parameter?(param) >> + end >> + >> + def self.metaparameters >> + Puppet::Type.parameters >> + end >> + >> + # All new code in this block >> + ############################# >> + >> + ############################# >> + # This code should probably all be removed >> # All parameters, in the appropriate order. The key_attributes come >> first, then >> # the provider, then the properties, and finally the params and metaparams >> # in the order they were specified in the files. >> def self.allattrs >> - key_attributes | (parameters & [:provider]) | properties.collect { >> |property| property.name } | parameters | metaparams >> + parameter_names >> + end >> + >> + def self.metaparams >> + metaparameters >> + end >> + >> + # Find the metaparameter class associated with a given metaparameter name. >> + def self.metaparamclass(name) >> + Puppet::Type.parameter(name) >> + end >> + >> + # Find a parameter, property, or metaparameter class by name >> + def self.parameter_class(name) >> + parameter(name) >> + end >> + >> + def self.properties >> + parameters.find_all { |p| p.property? } >> end >> >> # Find the class associated with any given attribute. >> def self.attrclass(name) >> - @attrclasses ||= {} >> - >> - # We cache the value, since this method gets called such a huge number >> - # of times (as in, hundreds of thousands in a given run). >> - unless @attrclasses.include?(name) >> - @attrclasses[name] = case self.attrtype(name) >> - when :property; @validproperties[name] >> - when :meta; @@metaparamhash[name] >> - when :param; @paramhash[name] >> - end >> - end >> - @attrclasses[name] >> + parameter(name) >> end >> >> # What type of parameter are we dealing with? Cache the results, because >> # this method gets called so many times. >> def self.attrtype(attr) >> - @attrtypes ||= {} >> - unless @attrtypes.include?(attr) >> - @attrtypes[attr] = case >> - when @validproperties.include?(attr); :property >> - when @paramhash.include?(attr); :param >> - when @@metaparamhash.include?(attr); :meta >> - end >> - end >> - >> - @attrtypes[attr] >> + parameter_type(attr) >> end >> >> def self.eachmetaparam >> - @@metaparams.each { |p| yield p.name } >> + parameters.each { |p| yield p.name } >> + end > > > Shouldn't eachmetaparam be > > Puppet::Type.parameters.each { |p| yield p.name } > > Metaparamters are parameters that are definied for the Puppet::Type > class instead of any possible derived classes. Probably, but in looking through my code, it looks like this method is actually removed later - I think this only existed as a temporary measure while I was gradually (one commit at a time) removing methods. >> + # Find the parameter class associated with a given parameter name. >> + def self.paramclass(name) >> + parameter(name) >> + end >> + >> + # Return the property class associated with a name >> + def self.propertybyname(name) >> + parameter(name) >> + end >> + >> + def self.validattr?(name) >> + valid_parameter?(name) >> + end >> + >> + # does the name reflect a valid property? >> + def self.validproperty?(name) >> + p = parameter(name) and p.property? >> + end >> + >> + # Return the list of validproperties >> + def self.validproperties >> + parameters.find_all { |p| p.property? }.collect { |p| p.name } >> end >> >> + # does the name reflect a valid parameter? >> + def self.validparameter?(name) >> + valid_aparameter?(name) >> + end > > > I dont know why we need a »validparameter?« and a »valid_parameter?« > method but I guess »valid_aparameter?« is just a typo. Another temporary change, so while you're right, it doesn't matter after it's all done. >> + # end of code likely to be removed >> + ############################## >> + >> # Create the 'ensure' class. This is a separate method so other types >> # can easily call it and create their own 'ensure' values. >> def self.ensurable(&block) >> @@ -267,40 +390,18 @@ class Type >> end >> end >> >> - # Is the parameter in question a meta-parameter? >> - def self.metaparam?(param) >> - @@metaparamhash.include?(symbolize(param)) >> - end >> - >> - # Find the metaparameter class associated with a given metaparameter name. >> - def self.metaparamclass(name) >> - @@metaparamhash[symbolize(name)] >> - end >> - >> - def self.metaparams >> - @@metaparams.collect { |param| param.name } >> - end >> - >> - def self.metaparamdoc(metaparam) >> - @@metaparamhash[metaparam].doc >> - end >> - >> # Create a new metaparam. Requires a block and a name, stores it in the >> - # @parameters array, and does some basic checking on it. >> + # @metaparams array, and does some basic checking on it. >> def self.newmetaparam(name, options = {}, &block) >> - @@metaparams ||= [] >> - @@metaparamhash ||= {} >> - name = symbolize(name) >> + raise "Only Puppet::Type can add metaparams" unless self == Puppet::Type >> >> + name = symbolize(name) >> >> - param = genclass( >> - name, >> + param = genclass( >> + name, >> :parent => options[:parent] || Puppet::Parameter, >> :prefix => "MetaParam", >> - :hash => @@metaparamhash, >> - :array => @@metaparams, >> :attributes => options[:attributes], >> - >> &block >> ) >> >> @@ -311,12 +412,15 @@ class Type >> >> param.metaparam = true >> >> + # Parameters stored by Puppet::Type are metaparams, by definition. >> + add_parameter_class(param) >> + >> param >> end >> >> def self.key_attribute_parameters >> @key_attribute_parameters ||= ( >> - params = @parameters.find_all { |param| >> + params = parameters.find_all { |param| >> param.isnamevar? or param.name == :name >> } >> ) >> @@ -342,7 +446,7 @@ class Type >> end >> >> # Create a new parameter. Requires a block and a name, stores it in the >> - # @parameters array, and does some basic checking on it. >> + # parameter list, and does some basic checking on it. >> def self.newparam(name, options = {}, &block) >> options[:attributes] ||= {} >> >> @@ -351,10 +455,7 @@ class Type >> :parent => options[:parent] || Puppet::Parameter, >> :attributes => options[:attributes], >> :block => block, >> - :prefix => "Parameter", >> - :array => @parameters, >> - >> - :hash => @paramhash >> + :prefix => "Parameter" >> ) >> >> handle_param_options(name, options) >> @@ -364,6 +465,8 @@ class Type >> >> param.isnamevar if options[:namevar] >> >> + add_parameter_class(param) >> + >> param >> end >> >> @@ -383,7 +486,7 @@ class Type >> "Options must be a hash, not #{options.inspect}" >> end >> >> - raise Puppet::DevError, "Class #{self.name} already has a property >> named #{name}" if @validproperties.include?(name) >> + raise Puppet::DevError, "Class #{self.name} already has a parameter >> named #{name}" if parameter(name) >> >> if parent = options[:parent] >> options.delete(:parent) >> @@ -394,7 +497,7 @@ class Type >> # We have to create our own, new block here because we want to define >> # an initial :retrieve method, if told to, and then eval the passed >> # block if available. >> - prop = genclass(name, :parent => parent, :hash => @validproperties, >> :attributes => options) do >> + prop = genclass(name, :parent => parent, :attributes => options) do >> # If they've passed a retrieve method, then override the retrieve >> # method on the class. >> if options[:retrieve] >> @@ -407,71 +510,11 @@ class Type >> end >> >> # If it's the 'ensure' property, always put it first. >> - if name == :ensure >> - @properties.unshift prop >> - else >> - @properties << prop >> - end >> + add_parameter_class(prop) >> >> prop >> end >> >> - def self.paramdoc(param) >> - @paramhash[param].doc >> - end >> - >> - # Return the parameter names >> - def self.parameters >> - return [] unless defined?(@parameters) >> - @parameters.collect { |klass| klass.name } >> - end >> - >> - # Find the parameter class associated with a given parameter name. >> - def self.paramclass(name) >> - @paramhash[name] >> - end >> - >> - # Return the property class associated with a name >> - def self.propertybyname(name) >> - @validproperties[name] >> - end >> - >> - def self.validattr?(name) >> - name = symbolize(name) >> - return true if name == :name >> - @validattrs ||= {} >> - >> - unless @validattrs.include?(name) >> - @validattrs[name] = !!(self.validproperty?(name) or >> self.validparameter?(name) or self.metaparam?(name)) >> - end >> - >> - @validattrs[name] >> - end >> - >> - # does the name reflect a valid property? >> - def self.validproperty?(name) >> - name = symbolize(name) >> - @validproperties.include?(name) && @validproperties[name] >> - end >> - >> - # Return the list of validproperties >> - def self.validproperties >> - return {} unless defined?(@parameters) >> - >> - @validproperties.keys >> - end >> - >> - # does the name reflect a valid parameter? >> - def self.validparameter?(name) >> - raise Puppet::DevError, "Class #{self} has not defined parameters" >> unless defined?(@parameters) >> - !!(@paramhash.include?(name) or @@metaparamhash.include?(name)) >> - end >> - >> - # This is a forward-compatibility method - it's the validity interface >> we'll use in Puppet::Resource. >> - def self.valid_parameter?(name) >> - validattr?(name) >> - end >> - >> # Are we deleting this resource? >> def deleting? >> obj = @parameters[:ensure] and obj.should == :absent >> @@ -578,7 +621,7 @@ class Type >> name = klass.name >> end >> >> - unless klass = self.class.attrclass(name) >> + unless klass = self.class.parameter(name) >> raise Puppet::Error, "Resource type #{self.class.name} does not >> support parameter #{name}" >> end >> >> @@ -618,8 +661,10 @@ class Type >> # For any parameters or properties that have defaults and have not yet been >> # set, set them now. This method can be handed a list of attributes, >> # and if so it will only set defaults for those attributes. >> - def set_default(attr) >> - return unless klass = self.class.attrclass(attr) >> + def set_default(klass) >> + unless klass.is_a?(Class) >> + return unless klass = self.class.parameter(klass) >> + end >> return unless klass.method_defined?(:default) >> return if @parameters.include?(klass.name) >> >> @@ -1464,7 +1509,7 @@ class Type >> # Make sure we have a :provider parameter defined. Only gets called if >> there >> # are providers. >> def self.providify >> - return if @paramhash.has_key? :provider >> + return if parameter(:provider) >> >> newparam(:provider) do >> desc "The specific backend for #{self.name.to_s} to use. You will >> @@ -1638,13 +1683,6 @@ class Type >> >> # all of the variables that must be initialized for each subclass >> def self.initvars >> - @parameters ||= [] >> - >> - @validproperties = {} >> - @properties = [] >> - @parameters = [] >> - @paramhash = {} >> - >> @doc ||= "" >> end >> >> @@ -1741,7 +1779,8 @@ class Type >> # extra attributes from the resource so we get failures >> # on invalid attributes. >> no_values = [] >> - (self.class.allattrs + hash.keys).uniq.each do |attr| >> + order = (self.class.allattrs + hash.keys).uniq >> + order.uniq.each do |attr| >> begin >> # Set any defaults immediately. This is mostly done so >> # that the default provider is available for any other >> diff --git a/spec/unit/parameter_spec.rb b/spec/unit/parameter_spec.rb >> index 04556c0..49dfdf1 100755 >> --- a/spec/unit/parameter_spec.rb >> +++ b/spec/unit/parameter_spec.rb >> @@ -21,6 +21,27 @@ describe Puppet::Parameter do >> @class.value_collection.should >> be_instance_of(Puppet::Parameter::ValueCollection) >> end >> >> + it "should default to being a 'parameter'" do >> + @class = Class.new(Puppet::Parameter) >> + @class.should be_parameter >> + @class.parameter_type.should == :parameter >> + end >> + >> + it "should be a property when it is a subclass of Property" do >> + @class = Class.new(Puppet::Property) >> + @class.should be_property >> + @class.parameter_type.should == :property >> + end >> + >> + it "should be a metaparameter when Puppet::Type thinks it is" do >> + @class = Class.new(Puppet::Parameter) do >> + @name = :foo >> + end >> + Puppet::Type.stubs(:metaparam?).with(:foo).returns true >> + @class.should be_metaparameter >> + @class.parameter_type.should == :metaparameter >> + end >> + >> it "should return its name as a string when converted to a string" do >> @parameter.to_s.should == @parameter.name.to_s >> end >> diff --git a/spec/unit/type_spec.rb b/spec/unit/type_spec.rb >> index 30c5511..a39dec1 100755 >> --- a/spec/unit/type_spec.rb >> +++ b/spec/unit/type_spec.rb >> @@ -6,18 +6,6 @@ describe Puppet::Type do >> Puppet::Type.ancestors.should be_include(Puppet::Util::Cacher) >> end >> >> - it "should consider a parameter to be valid if it is a valid parameter" do >> - Puppet::Type.type(:mount).should be_valid_parameter(:path) >> - end >> - >> - it "should consider a parameter to be valid if it is a valid property" do >> - Puppet::Type.type(:mount).should be_valid_parameter(:fstype) >> - end >> - >> - it "should consider a parameter to be valid if it is a valid metaparam" do >> - Puppet::Type.type(:mount).should be_valid_parameter(:noop) >> - end >> - >> it "should use its catalog as its expirer" do >> catalog = Puppet::Resource::Catalog.new >> resource = Puppet::Type.type(:mount).new(:name => "foo", :fstype => >> "bar", :pass => 1, :ensure => :present) >> @@ -138,6 +126,143 @@ describe Puppet::Type do >> resource.should_not be_noop >> end >> >> + describe "when defining parameters" do >> + before do >> + @type = Puppet::Type.newtype(:parameter_tester) >> + end >> + >> + after do >> + Puppet::Type.rmtype(:parameter_tester) >> + end >> + >> + it "should support defining and retrieving parameters" do >> + @type.newparam(:foo) >> + @type.parameter(:foo).should be_instance_of(Class) >> + end >> + >> + it "should support retrieving parameters specified with a string" do >> + @type.newparam(:foo) >> + @type.parameter("foo").should be_instance_of(Class) >> + end >> + >> + it "should support returning all parameters" do >> + foo = @type.newparam(:foo) >> + @type.parameters.should be_include(foo) >> + end >> + >> + it "should always return parameters in the order specified" do >> + foo = @type.newparam(:foo) >> + bar = @type.newparam(:bar) >> + baz = @type.newparam(:baz) >> + params = @type.parameters >> + params.index(foo).should < params.index(bar) >> + params.index(bar).should < params.index(baz) >> + end >> + >> + it "should always put the namevar first" do >> + foo = @type.newparam(:foo) >> + name = @type.newparam(:name) { isnamevar } >> + params = @type.parameters >> + params.index(name).should < params.index(foo) >> + end >> + >> + it "should always put the namevar first even if the parameter isn't >> declared the namevar but is named 'name'" do >> + foo = @type.newparam(:foo) >> + name = @type.newparam(:name) >> + params = @type.parameters >> + params.index(name).should < params.index(foo) >> + end >> + >> + it "should always put 'provider' as the first-non-namevar parameter" do >> + foo = @type.newparam(:foo) >> + provider = @type.newparam(:provider) >> + name = @type.newparam(:name) >> + params = @type.parameters >> + params.index(name).should < params.index(provider) >> + params.index(provider).should < params.index(foo) >> + end >> + >> + it "should always put 'provider' as the first-non-namevar parameter >> even when it's added later" do >> + name = @type.newparam(:name) >> + provider = @type.newparam(:provider) >> + foo = @type.newparam(:foo) >> + params = @type.parameters >> + params.index(name).should < params.index(provider) >> + params.index(provider).should < params.index(foo) >> + end >> + >> + it "should always put 'ensure' before any other properties but after >> the namevar" do >> + foo = @type.newproperty(:foo) >> + ens = @type.newproperty(:ensure) >> + name = @type.newparam(:name) { isnamevar } >> + params = @type.parameters >> + params.index(name).should < params.index(ens) >> + params.index(ens).should < params.index(foo) >> + end >> + >> + it "should include metaparameters when asked for all parameters" do >> + noop = @type.parameter(:noop) >> + @type.parameters.should be_include(noop) >> + end >> + >> + it "should be able to return a list of parameter names" do >> + @type.newparam(:foo) >> + @type.parameter_names.should be_include(:foo) >> + end >> + >> + it "should include metaparameters when asked for all parameter names" do >> + @type.parameter(:noop) >> + @type.parameter_names.should be_include(:noop) >> + end >> + >> + it "should support defining and retrieving properties" do >> + @type.newproperty(:foo) >> + @type.parameter(:foo).should be_instance_of(Class) >> + end >> + >> + it "should be able to retrieve a metaparameter class by name" do >> + @type.parameter(:noop).should be_instance_of(Class) >> + end >> + >> + it "should consider subclasses of Property to be properties" do >> + @type.newproperty(:foo) >> + @type.parameter_type(:foo).should == :property >> + end >> + >> + it "should be able to determine parameter type of parameters passed as >> a string" do >> + @type.newproperty(:foo) >> + @type.parameter_type("foo").should == :property >> + end >> + >> + it "should be able to detect metaparameters" do >> + @type.parameter_type(:noop).should == :metaparameter >> + end >> + >> + it "should consider any non-metaparam subclass of Parameter to be a >> parameter" do >> + @type.newparam(:foo) >> + @type.parameter_type(:foo).should == :parameter >> + end >> + >> + it "should consider a defined parameter to be valid" do >> + @type.newparam(:foo) >> + @type.should be_valid_parameter(:foo) >> + end >> + >> + it "should consider a defined property to be valid" do >> + @type.newproperty(:foo) >> + @type.should be_valid_parameter(:foo) >> + end >> + >> + it "should consider metaparameters to be valid" do >> + @type.should be_valid_parameter(:noop) >> + end >> + >> + it "should accept parameters specified as a string" do >> + @type.newparam(:foo) >> + @type.should be_valid_parameter("foo") >> + end >> + end >> + >> describe "when creating an event" do >> before do >> @resource = Puppet::Type.type(:mount).new :name => "foo" >> @@ -202,7 +327,7 @@ describe Puppet::Type do >> it "should ensure its type has a 'provider' parameter" do >> @type.provide(:test_provider) >> >> - @type.parameters.should include(:provider) >> + @type.parameter_names.should include(:provider) >> end >> >> it "should remove a previously registered provider with the same name" do >> @@ -675,7 +800,7 @@ describe Puppet::Type.metaparamclass(:audit) do >> newparam(:name) {} >> end >> >> - type.parameters.should include(:provider) >> + type.parameter_names.should include(:provider) >> end >> end >> end >> diff --git a/test/ral/manager/attributes.rb b/test/ral/manager/attributes.rb >> index caa90d5..d01e35a 100755 >> --- a/test/ral/manager/attributes.rb >> +++ b/test/ral/manager/attributes.rb >> @@ -113,7 +113,11 @@ class TestTypeAttributes < Test::Unit::TestCase >> :meta => :newmetaparam, :param => :newparam, :prop => :newproperty >> }.each do |name, method| >> assert_nothing_raised("Could not make #{name} of type #{method}") do >> - type.send(method, name) {} >> + if method == :newmetaparam >> + Puppet::Type.send(method, name) {} >> + else >> + type.send(method, name) {} >> + end >> end >> end >> >> @@ -170,6 +174,7 @@ class TestTypeAttributes < Test::Unit::TestCase >> [nope, maybe, yep].each_with_index do |prov, i| >> resource = type.new(:provider => prov.name, :name => "test#{i}", :none >> => "a", :one => "b", :two => "c") >> >> + >> case prov.name >> when :nope >> yes = [:none] >> @@ -181,9 +186,9 @@ class TestTypeAttributes < Test::Unit::TestCase >> yes = [:none, :one, :two] >> no = [] >> end >> - yes.each { |a| assert(resource.should(a), "Did not get value for #{a} >> in #{prov.name}") } >> + yes.each { |a| assert(resource[a], "Did not get value for '#{a}' in >> '#{prov.name}'") } >> no.each do |a| >> - assert_nil(resource.should(a), "Got value for unsupported %s in %s" >> % [a, prov.name]) >> + assert_nil(resource[a], "Got value for unsupported %s in %s" % [a, >> prov.name]) >> if Puppet::Util::Log.sendlevel?(:info) >> assert(@logs.find { |l| l.message =~ /not managing attribute #{a}/ >> and l.level == :info }, "No warning about failed %s" % a) >> end >> @@ -222,33 +227,4 @@ class TestTypeAttributes < Test::Unit::TestCase >> inst.value = :nosuchattr >> end >> end >> - >> - def test_check_ignores_unsupported_params >> - type = Puppet::Type.newtype(:unsupported) do >> - feature :nosuchfeat, "testing" >> - newparam(:name) {} >> - newproperty(:yep) {} >> - newproperty(:nope, :required_features => :nosuchfeat) {} >> - end >> - $yep = :absent >> - type.provide(:only) do >> - def self.supports_parameter?(param) >> - param.name != :nope >> - end >> - >> - def yep >> - $yep >> - end >> - def yep=(v) >> - $yep = v >> - end >> - end >> - cleanup { Puppet::Type.rmtype(:unsupported) } >> - >> - obj = type.new(:name => "test", :check => :yep) >> - obj.stubs(:newattr).returns(stub_everything("newattr")) >> - obj.expects(:newattr).with(:nope).never >> - obj[:check] = :all >> - end >> end >> - >> diff --git a/test/ral/manager/type.rb b/test/ral/manager/type.rb >> index c2e6a0c..d8995e2 100755 >> --- a/test/ral/manager/type.rb >> +++ b/test/ral/manager/type.rb >> @@ -204,7 +204,7 @@ class TestType < Test::Unit::TestCase >> end >> >> # Now create an instance >> - obj = type.create(:name => :myobj) >> + obj = type.new(:name => :myobj) >> >> inst = property.new(:resource => obj) >> >> @@ -241,7 +241,7 @@ class TestType < Test::Unit::TestCase >> path = tempfile >> assert_nothing_raised do >> >> - file = fileclass.create( >> + file = fileclass.new( >> >> :title => "Myfile", >> >> @@ -257,7 +257,7 @@ class TestType < Test::Unit::TestCase >> # Now make sure we can specify both and still get the right answers >> assert_nothing_raised do >> >> - file = fileclass.create( >> + file = fileclass.new( >> >> :title => "Myfile", >> >> diff --git a/test/ral/providers/provider.rb b/test/ral/providers/provider.rb >> index a9f5ad2..446f25f 100755 >> --- a/test/ral/providers/provider.rb >> +++ b/test/ral/providers/provider.rb >> @@ -226,8 +226,8 @@ class TestProvider < Test::Unit::TestCase >> >> def test_mk_resource_methods >> prov = newprovider >> - resourcetype = Struct.new(:validproperties, :parameters) >> - m = resourcetype.new([:prop1, :prop2], [:param1, :param2]) >> + resourcetype = Struct.new(:parameter_names) >> + m = resourcetype.new([:prop1, :prop2, :param1, :param2]) >> prov.resource_type = m >> >> assert_nothing_raised("could not call mk_resource_methods") do >> diff --git a/test/ral/type/exec.rb b/test/ral/type/exec.rb >> index 0831d46..1d4296e 100755 >> --- a/test/ral/type/exec.rb >> +++ b/test/ral/type/exec.rb >> @@ -302,6 +302,7 @@ class TestExec < Test::Unit::TestCase >> >> assert_events([:executed_command], comp) >> assert_events([:executed_command], comp) >> + assert_events([:executed_command], comp) >> system("touch #{afile}") >> assert_events([], comp) >> assert_events([], comp) >> diff --git a/test/ral/type/file/target.rb b/test/ral/type/file/target.rb >> index d778f28..4554aba 100755 >> --- a/test/ral/type/file/target.rb >> +++ b/test/ral/type/file/target.rb >> @@ -176,7 +176,7 @@ class TestFileTarget < Test::Unit::TestCase >> source = tempfile >> dest = tempfile >> >> - obj = @file.create(:path => source, :target => dest) >> + obj = @file.new(:path => source, :target => dest) >> >> prop = obj.send(:property, :target) >> prop.send(:instance_variable_set, "@should", [:nochange]) >> diff --git a/test/ral/type/resources.rb b/test/ral/type/resources.rb >> index fcfeebe..704d138 100755 >> --- a/test/ral/type/resources.rb >> +++ b/test/ral/type/resources.rb >> @@ -22,7 +22,7 @@ class TestResources < Test::Unit::TestCase >> def mk_purger(managed = false) >> @purgenum ||= 0 >> @purgenum += 1 >> - obj = @purgetype.create :name => "purger#{@purgenum}" >> + obj = @purgetype.new :name => "purger#{@purgenum}" >> $purgemembers[obj[:name]] = obj >> obj[:fake] = "testing" if managed >> obj >> @@ -79,8 +79,8 @@ class TestResources < Test::Unit::TestCase >> # Now make sure root fails the test >> @user = Puppet::Type.type(:user) >> assert_nothing_raised { >> - assert(! res.check(@user.create(:name => "root")), "root passed >> check") >> - assert(! res.check(@user.create(:name => "nobody")), "nobody passed >> check") >> + assert(! res.check(@user.new(:name => "root")), "root passed check") >> + assert(! res.check(@user.new(:name => "nobody")), "nobody passed >> check") >> } >> >> # Now find a user between 0 and the limit >> @@ -98,10 +98,10 @@ class TestResources < Test::Unit::TestCase >> end >> } >> >> - assert(! res.check(@user.create(:name => low)), "low user #{low} passed >> check") if low >> + assert(! res.check(@user.new(:name => low)), "low user #{low} passed >> check") if low >> if high >> res[:unless_system_user] = 50 >> - assert(res.check(@user.create(:name => high)), "high user #{high} >> failed check") >> + assert(res.check(@user.new(:name => high)), "high user #{high} failed >> check") >> end >> end >> end >> -- >> 1.7.3.1 >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Puppet Developers" group. >> To post to this group, send email to [email protected]. >> To unsubscribe from this group, send email to >> [email protected]. >> For more options, visit this group at >> http://groups.google.com/group/puppet-dev?hl=en. >> -- Luke Kanies | http://about.me/lak | http://puppetlabs.com/ | +1-615-594-8199 -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
