Please review pull request #568: (#12960) fix order dependent test failures over global state opened by (daniel-pittman)
Description:
We had some order dependent test failures, caused by a type named foo being defined.
Because we don't clean up global state to revoke those named types, we wind up in the position that we broke a bunch of other places that also created something named foo overlapping the original - but only if they ran in the right order.
(Which, unfortunately, still turns out to be "on CI, but not my workstation")
- Opened: Thu Mar 08 19:58:05 UTC 2012
- Based on: puppetlabs:2.7.x (dad6f17281771747cac246e53814b5d6bea0ac26)
- Requested merge: daniel-pittman:maint/2.7.x/12960-fix-order-dependent-test-failures-over-global-state (40ee256f15d60dc990e312567b59672101ac6818)
Diff follows:
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 867936c..e416665 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -123,6 +123,13 @@ def self.ensurable?
}
end
+ # These `apply_to` methods are horrible. They should really be implemented
+ # as part of the usual system of constraints that apply to a type and
+ # provider pair, but were implemented as a separate shadow system.
+ #
+ # We should rip them out in favour of a real constraint pattern around the
+ # target device - whatever that looks like - and not have this additional
+ # magic here. --daniel 2012-03-08
def self.apply_to_device
@apply_to = :device
end
diff --git a/spec/unit/parser/functions/create_resources_spec.rb b/spec/unit/parser/functions/create_resources_spec.rb
index 64314f7..0eca718 100755
--- a/spec/unit/parser/functions/create_resources_spec.rb
+++ b/spec/unit/parser/functions/create_resources_spec.rb
@@ -20,16 +20,19 @@ def get_scope
it "should exist" do
Puppet::Parser::Functions.function(:create_resources).should == "function_create_resources"
end
+
it 'should require two or three arguments' do
- lambda { @scope.function_create_resources(['foo']) }.should raise_error(ArgumentError, 'create_resources(): wrong number of arguments (1; must be 2 or 3)')
- lambda { @scope.function_create_resources(['foo', 'bar', 'blah', 'baz']) }.should raise_error(ArgumentError, 'create_resources(): wrong number of arguments (4; must be 2 or 3)')
+ expect { @scope.function_create_resources(['foo']) }.should raise_error(ArgumentError, 'create_resources(): wrong number of arguments (1; must be 2 or 3)')
+ expect { @scope.function_create_resources(['foo', 'bar', 'blah', 'baz']) }.should raise_error(ArgumentError, 'create_resources(): wrong number of arguments (4; must be 2 or 3)')
end
+
describe 'when creating native types' do
before :each do
Puppet[:code]='notify{test:}'
get_scope
@scope.resource=Puppet::Parser::Resource.new('class', 't', :scope => @scope)
end
+
it 'empty hash should not cause resources to be added' do
@scope.function_create_resources(['file', {}])
@compiler.catalog.resources.size == 1
@@ -47,7 +50,7 @@ def get_scope
@compiler.catalog.resource(:notify, "bar")['message'].should == 'two'
end
it 'should fail to add non-existing type' do
- lambda { @scope.function_create_resources(['foo', {}]) }.should raise_error(ArgumentError, 'could not create resource of unknown type foo')
+ expect { @scope.function_create_resources(['create-resource-foo', {}]) }.should raise_error(ArgumentError, 'could not create resource of unknown type create-resource-foo')
end
it 'should be able to add edges' do
@scope.function_create_resources(['notify', {'foo'=>{'require' => 'Notify[test]'}}])
@@ -68,7 +71,7 @@ def get_scope
describe 'when dynamically creating resource types' do
before :each do
Puppet[:code]=
-'define foo($one){notify{$name: message => $one}}
+'define foocreateresource($one){notify{$name: message => $one}}
notify{test:}
'
get_scope
@@ -76,21 +79,21 @@ def get_scope
Puppet::Parser::Functions.function(:create_resources)
end
it 'should be able to create defined resoure types' do
- @scope.function_create_resources(['foo', {'blah'=>{'one'=>'two'}}])
+ @scope.function_create_resources(['foocreateresource', {'blah'=>{'one'=>'two'}}])
# still have to compile for this to work...
# I am not sure if this constraint ruins the tests
@scope.compiler.compile
@compiler.catalog.resource(:notify, "blah")['message'].should == 'two'
end
it 'should fail if defines are missing params' do
- @scope.function_create_resources(['foo', {'blah'=>{}}])
- lambda { @scope.compiler.compile }.should raise_error(Puppet::ParseError, 'Must pass one to Foo[blah] at line 1')
+ @scope.function_create_resources(['foocreateresource', {'blah'=>{}}])
+ expect { @scope.compiler.compile }.should raise_error(Puppet::ParseError, 'Must pass one to Foocreateresource[blah] at line 1')
end
it 'should be able to add multiple defines' do
hash = {}
hash['blah'] = {'one' => 'two'}
hash['blaz'] = {'one' => 'three'}
- @scope.function_create_resources(['foo', hash])
+ @scope.function_create_resources(['foocreateresource', hash])
# still have to compile for this to work...
# I am not sure if this constraint ruins the tests
@scope.compiler.compile
@@ -98,7 +101,7 @@ def get_scope
@compiler.catalog.resource(:notify, "blaz")['message'].should == 'three'
end
it 'should be able to add edges' do
- @scope.function_create_resources(['foo', {'blah'=>{'one'=>'two', 'require' => 'Notify[test]'}}])
+ @scope.function_create_resources(['foocreateresource', {'blah'=>{'one'=>'two', 'require' => 'Notify[test]'}}])
@scope.compiler.compile
rg = @scope.compiler.catalog.to_ral.relationship_graph
test = rg.vertices.find { |v| v.title == 'test' }
@@ -110,7 +113,7 @@ def get_scope
@compiler.catalog.resource(:notify, "blah")['message'].should == 'two'
end
it 'should account for default values' do
- @scope.function_create_resources(['foo', {'blah'=>{}}, {'one' => 'two'}])
+ @scope.function_create_resources(['foocreateresource', {'blah'=>{}}, {'one' => 'two'}])
@scope.compiler.compile
@compiler.catalog.resource(:notify, "blah")['message'].should == 'two'
end
@@ -129,10 +132,10 @@ def get_scope
@scope.function_create_resources(['class', {'bar'=>{'one'=>'two'}}])
@scope.compiler.compile
@compiler.catalog.resource(:notify, "test")['message'].should == 'two'
- @compiler.catalog.resource(:class, "bar").should_not be_nil#['message'].should == 'two'
+ @compiler.catalog.resource(:class, "bar").should_not be_nil
end
it 'should fail to create non-existing classes' do
- lambda { @scope.function_create_resources(['class', {'blah'=>{'one'=>'two'}}]) }.should raise_error(ArgumentError ,'could not find hostclass blah')
+ expect { @scope.function_create_resources(['class', {'blah'=>{'one'=>'two'}}]) }.should raise_error(ArgumentError ,'could not find hostclass blah')
end
it 'should be able to add edges' do
@scope.function_create_resources(['class', {'bar'=>{'one'=>'two', 'require' => 'Notify[tester]'}}])
@@ -148,7 +151,7 @@ def get_scope
@scope.function_create_resources(['class', {'bar'=>{}}, {'one' => 'two'}])
@scope.compiler.compile
@compiler.catalog.resource(:notify, "test")['message'].should == 'two'
- @compiler.catalog.resource(:class, "bar").should_not be_nil#['message'].should == 'two'
+ @compiler.catalog.resource(:class, "bar").should_not be_nil
end
end
end
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index 7c2c6ac..aedd25f 100755
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -5,9 +5,7 @@
describe Puppet::Resource do
include PuppetSpec::Files
- before do
- @basepath = make_absolute("/somepath")
- end
+ let :basepath do make_absolute("/somepath") end
[:catalog, :file, :line].each do |attr|
it "should have an #{attr} attribute" do
@@ -87,27 +85,27 @@
end
it "should be able to extract its information from a Puppet::Type instance" do
- ral = Puppet::Type.type(:file).new :path => @basepath+"/foo"
+ ral = Puppet::Type.type(:file).new :path => basepath+"/foo"
ref = Puppet::Resource.new(ral)
ref.type.should == "File"
- ref.title.should == @basepath+"/foo"
+ ref.title.should == basepath+"/foo"
end
it "should fail if the title is nil and the type is not a valid resource reference string" do
- lambda { Puppet::Resource.new("foo") }.should raise_error(ArgumentError)
+ expect { Puppet::Resource.new("resource-spec-foo") }.should raise_error(ArgumentError)
end
it 'should fail if strict is set and type does not exist' do
- lambda { Puppet::Resource.new('foo', 'title', {:strict=>true}) }.should raise_error(ArgumentError, 'Invalid resource type foo')
+ expect { Puppet::Resource.new('resource-spec-foo', 'title', {:strict=>true}) }.should raise_error(ArgumentError, 'Invalid resource type resource-spec-foo')
end
it 'should fail if strict is set and class does not exist' do
- lambda { Puppet::Resource.new('Class', 'foo', {:strict=>true}) }.should raise_error(ArgumentError, 'Could not find declared class foo')
+ expect { Puppet::Resource.new('Class', 'resource-spec-foo', {:strict=>true}) }.should raise_error(ArgumentError, 'Could not find declared class resource-spec-foo')
end
it "should fail if the title is a hash and the type is not a valid resource reference string" do
- expect { Puppet::Resource.new({:type => "foo", :title => "bar"}) }.
+ expect { Puppet::Resource.new({:type => "resource-spec-foo", :title => "bar"}) }.
to raise_error ArgumentError, /Puppet::Resource.new does not take a hash/
end
@@ -286,11 +284,11 @@
end
it "should fail if invalid parameters are used" do
- lambda { Puppet::Resource.new("file", "/path", :strict => true, :parameters => {:nosuchparam => "bar"}) }.should raise_error
+ expect { Puppet::Resource.new("file", "/path", :strict => true, :parameters => {:nosuchparam => "bar"}) }.should raise_error
end
it "should fail if the resource type cannot be resolved" do
- lambda { Puppet::Resource.new("nosuchtype", "/path", :strict => true) }.should raise_error
+ expect { Puppet::Resource.new("nosuchtype", "/path", :strict => true) }.should raise_error
end
end
@@ -355,7 +353,7 @@
it "should be able to set the name for non-builtin types" do
resource = Puppet::Resource.new(:foo, "bar")
resource[:name] = "eh"
- lambda { resource[:name] = "eh" }.should_not raise_error
+ expect { resource[:name] = "eh" }.should_not raise_error
end
it "should be able to return the name for non-builtin types" do
@@ -472,7 +470,7 @@
end
it "should deserialize a Puppet::Resource::Reference without exceptions" do
- lambda { YAML.load(@old_storedconfig_yaml) }.should_not raise_error
+ expect { YAML.load(@old_storedconfig_yaml) }.should_not raise_error
end
it "should deserialize as a Puppet::Resource::Reference as a Puppet::Resource" do
@@ -486,10 +484,10 @@
describe "when converting to a RAL resource" do
it "should use the resource type's :new method to create the resource if the resource is of a builtin type" do
- resource = Puppet::Resource.new("file", @basepath+"/my/file")
+ resource = Puppet::Resource.new("file", basepath+"/my/file")
result = resource.to_ral
result.should be_instance_of(Puppet::Type.type(:file))
- result[:path].should == @basepath+"/my/file"
+ result[:path].should == basepath+"/my/file"
end
it "should convert to a component instance if the resource type is not of a builtin type" do
@@ -526,7 +524,7 @@
describe "when converting to a TransObject" do
describe "and the resource is not an instance of a builtin type" do
before do
- @resource = Puppet::Resource.new("foo", "bar")
+ @resource = Puppet::Resource.new("resource-spec-foo", "bar")
end
it "should return a simple TransBucket if it is not an instance of a builtin type" do
@@ -697,7 +695,7 @@ def pson_result_should
before do
@data = {
'type' => "file",
- 'title' => @basepath+"/yay",
+ 'title' => basepath+"/yay",
}
end
@@ -706,7 +704,7 @@ def pson_result_should
end
it "should set its title to the provided title" do
- Puppet::Resource.from_pson(@data).title.should == @basepath+"/yay"
+ Puppet::Resource.from_pson(@data).title.should == basepath+"/yay"
end
it "should tag the resource with any provided tags" do
@@ -737,12 +735,12 @@ def pson_result_should
it "should fail if no title is provided" do
@data.delete('title')
- lambda { Puppet::Resource.from_pson(@data) }.should raise_error(ArgumentError)
+ expect { Puppet::Resource.from_pson(@data) }.should raise_error(ArgumentError)
end
it "should fail if no type is provided" do
@data.delete('type')
- lambda { Puppet::Resource.from_pson(@data) }.should raise_error(ArgumentError)
+ expect { Puppet::Resource.from_pson(@data) }.should raise_error(ArgumentError)
end
it "should set each of the provided parameters" do
diff --git a/spec/unit/type_spec.rb b/spec/unit/type_spec.rb
index 1f7d10d..1e3f46d 100755
--- a/spec/unit/type_spec.rb
+++ b/spec/unit/type_spec.rb
@@ -612,22 +612,21 @@
end
describe "::instances" do
- before :each do
- @fake_type = Puppet::Type.newtype(:foo) do
+ it "should not fail if no suitable providers are found" do
+ fake_type = Puppet::Type.newtype(:type_spec_fake_type) do
newparam(:name) do
isnamevar
end
newproperty(:prop1) do
end
+
+ provide(:fake1) do
+ confine :exists => '/no/such/file'
+ mk_resource_methods
+ end
end
- @unsuitable_provider = Puppet::Type.type(:foo).provide(:fake1) do
- confine :exists => '/no/such/file'
- mk_resource_methods
- end
- end
- it "should not fail if no suitable providers are found" do
- lambda { @fake_type.instances }.should_not raise_error
+ expect { fake_type.instances }.should_not raise_error
end
end
-- 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.
