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
+
+  # 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
+  # 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.

Reply via email to