Hi Luke, I'm currently reviewing the patchset. One general question: Is this targeted to a specific release or ready to merge?
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) 2 specific comments inline. 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. > + > + # 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. > + # 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. >
pgpiz5UsCai3t.pgp
Description: PGP signature
