Please review pull request #697: Feature/13970/remove dynamic scoping opened by (zaphod42)

Description:

Removal of dynamic scoping was slated for Telly, this removes the dynamic scoping and adds some test cases around various scoping cases.

There is also a fix for a problem that was noticed during this work for the += semantics. The += operator should not modify the value of the variable that it is appending to, but this did not hold true for arrays. This should now be fixed.

  • Opened: Fri Apr 20 21:04:42 UTC 2012
  • Based on: puppetlabs:master (54e1c83afde1b721e5433adea17a4dd0caffbc63)
  • Requested merge: zaphod42:feature/13970/remove-dynamic-scoping (cc7c13eda8aabf7983e2ca4544e6ccd5f09d7afd)

Diff follows:

diff --git a/acceptance/tests/language/ticket_8174_enc_causes_spurious_deprecation_warnings.rb b/acceptance/tests/language/ticket_8174_enc_causes_spurious_deprecation_warnings.rb
deleted file mode 100644
index c29e5bf..0000000
--- a/acceptance/tests/language/ticket_8174_enc_causes_spurious_deprecation_warnings.rb
+++ /dev/null
@@ -1,80 +0,0 @@
-test_name "#8174: incorrect warning about deprecated scoping"
-
-testdir = master.tmpdir('scoping_deprecation')
-
-create_remote_file(master, "#{testdir}/puppet.conf", <<END)
-[main]
-node_terminus = exec
-external_nodes = "#{testdir}/enc"
-manifest = "#{testdir}/site.pp"
-modulepath = "#{testdir}/modules"
-END
-
-on master, "mkdir -p #{testdir}/modules/a/manifests"
-
-create_remote_file(master, "#{testdir}/enc", <<-PP)
-#!/usr/bin/env sh
-
-cat <<END
----
-classes:
-  a
-parameters:
-  enc_var: "Set from ENC."
-END
-exit 0
-PP
-
-create_remote_file(master, "#{testdir}/site.pp", <<-PP)
-$top_scope = "set from site.pp"
-node default {
-  $node_var = "in node"
-}
-PP
-create_remote_file(master, "#{testdir}/modules/a/manifests/init.pp", <<-PP)
-class a {
-  $locally = "locally declared"
-  $dynamic_for_b = "dynamic and declared in a"
-  notify { "fqdn from facts": message => $fqdn }
-  notify { "locally declared var": message => $locally }
-  notify { "var via enc": message => $enc_var }
-  notify { "declared top scope": message => $top_scope }
-  notify { "declared node": message => $node_var }
-
-  include a::b
-}
-PP
-create_remote_file(master, "#{testdir}/modules/a/manifests/b.pp", <<-PP)
-class a::b {
-  notify { "dynamic from elsewhere": message => $dynamic_for_b }
-}
-PP
-
-on master, "chown -R root:puppet #{testdir}"
-on master, "chmod -R g+rwX #{testdir}"
-on master, "chmod -R a+x #{testdir}/enc"
-on master, "touch #{testdir}/log"
-on master, "chown puppet #{testdir}/log"
-
-assert_log_on_master_contains = lambda do |string|
-  on master, "grep '#{string}' #{testdir}/log"
-end
-
-assert_log_on_master_does_not_contain = lambda do |string|
-  on master, "grep -v '#{string}' #{testdir}/log"
-end
-
-with_master_running_on(master, "--config #{testdir}/puppet.conf --debug --verbose --daemonize --dns_alt_names=\"puppet,$(hostname -s),$(hostname -f)\" --autosign true --logdest #{testdir}/log") do
-  agents.each do |agent|
-    run_agent_on(agent, "--no-daemonize --onetime --server #{master}")
-  end
-
-  assert_log_on_master_contains['Dynamic lookup of $dynamic_for_b']
-  assert_log_on_master_does_not_contain['Dynamic lookup of $fqdn']
-  assert_log_on_master_does_not_contain['Dynamic lookup of $locally']
-  assert_log_on_master_does_not_contain['Dynamic lookup of $enc_var']
-  assert_log_on_master_does_not_contain['Dynamic lookup of $top_scope']
-  assert_log_on_master_does_not_contain['Dynamic lookup of $node_var']
-end
-
-on master, "rm -rf #{testdir}"
diff --git a/acceptance/tests/ticket_5027_warn_on_dynamic_scope.rb b/acceptance/tests/ticket_5027_warn_on_dynamic_scope.rb
deleted file mode 100644
index a918f07..0000000
--- a/acceptance/tests/ticket_5027_warn_on_dynamic_scope.rb
+++ /dev/null
@@ -1,28 +0,0 @@
-test_name "#5027: Issue warnings when using dynamic scope"
-
-step "Apply dynamic scoping manifest on agents"
-apply_manifest_on agents, %q{
-  $foo = 'foo_value'
-
-  class a {
-      $bar = 'bar_value'
-
-      include b
-  }
-
-  class b inherits c {
-      notify { $baz: } # should not generate a warning -- inherited from class c
-      notify { $bar: } # should generate a warning -- uses dynamic scoping
-      notify { $foo: } # should not generate a warning -- comes from top scope
-  }
-
-  class c {
-      $baz = 'baz_value'
-  }
-
-  include a
-}
-
-step "Verify deprecation warning"
-fail_test "Deprecation warning not issued" unless
-  stdout.include? 'warning: Dynamic lookup'
diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb
index 038b43f..cd851cf 100644
--- a/lib/puppet/parser/scope.rb
+++ b/lib/puppet/parser/scope.rb
@@ -110,7 +110,6 @@ def add_namespace(ns)
     end
   end
 
-  # Remove this when rebasing
   def environment
     compiler ? compiler.environment : Puppet::Node::Environment.new
   end
@@ -211,8 +210,6 @@ def lookupdefaults(type)
       }
     end
 
-    #Puppet.debug "Got defaults for %s: %s" %
-    #    [type,values.inspect]
     values
   end
 
@@ -229,71 +226,36 @@ def undef_as(x,v)
     end
   end
 
-  def qualified_scope(classname)
-    raise "class #{classname} could not be found"     unless klass = find_hostclass(classname)
-    raise "class #{classname} has not been evaluated" unless kscope = class_scope(klass)
-    kscope
-  end
-
-  private :qualified_scope
-
-  # Look up a variable with traditional scoping and then with new scoping. If
-  # the answers differ then print a deprecation warning.
   def lookupvar(name, options = {})
-    dynamic_value = dynamic_lookupvar(name,options)
-    twoscope_value = twoscope_lookupvar(name,options)
-    if dynamic_value != twoscope_value
-      location = (options[:file] && options[:line]) ? " at #{options[:file]}:#{options[:line]}" : ''
-      Puppet.deprecation_warning "Dynamic lookup of $#{name}#{location} is deprecated.  Support will be removed in a later version of Puppet.  Use a fully-qualified variable name (e.g., $classname::variable) or parameterized classes."
-    end
-    dynamic_value
-  end
-
-  # Look up a variable.  The simplest value search we do.
-  def twoscope_lookupvar(name, options = {})
     # Save the originating scope for the request
     options[:origin] = self unless options[:origin]
     table = ephemeral?(name) ? @ephemeral.last : @symtable
     if name =~ /^(.*)::(.+)$/
       begin
-        qualified_scope($1).twoscope_lookupvar($2, options.merge({:origin => nil}))
+        qualified_scope($1).lookupvar($2, options.merge({:origin => nil}))
       rescue RuntimeError => e
         location = (options[:file] && options[:line]) ? " at #{options[:file]}:#{options[:line]}" : ''
+        warning "Could not look up qualified variable '#{name}'; #{e.message}#{location}"
         nil
       end
     # If the value is present and either we are top/node scope or originating scope...
     elsif (ephemeral_include?(name) or table.include?(name)) and (compiler and self == compiler.topscope or (self.resource and self.resource.type == "Node") or self == options[:origin])
       table[name]
     elsif resource and resource.type == "Class" and parent_type = resource.resource_type.parent
-      class_scope(parent_type).twoscope_lookupvar(name,options.merge({:origin => nil}))
+      class_scope(parent_type).lookupvar(name,options.merge({:origin => nil}))
     elsif parent
-      parent.twoscope_lookupvar(name, options)
+      parent.lookupvar(name, options)
     else
       nil
     end
   end
 
-  # Look up a variable.  The simplest value search we do.
-  def dynamic_lookupvar(name, options = {})
-    table = ephemeral?(name) ? @ephemeral.last : @symtable
-    # If the variable is qualified, then find the specified scope and look the variable up there instead.
-    if name =~ /^(.*)::(.+)$/
-      begin
-        qualified_scope($1).dynamic_lookupvar($2,options)
-      rescue RuntimeError => e
-        location = (options[:file] && options[:line]) ? " at #{options[:file]}:#{options[:line]}" : ''
-        warning "Could not look up qualified variable '#{name}'; #{e.message}#{location}"
-        nil
-      end
-    elsif ephemeral_include?(name) or table.include?(name)
-      # We can't use "if table[name]" here because the value might be false
-      table[name]
-    elsif parent
-      parent.dynamic_lookupvar(name,options)
-    else
-      nil
-    end
+  def qualified_scope(classname)
+    raise "class #{classname} could not be found"     unless klass = find_hostclass(classname)
+    raise "class #{classname} has not been evaluated" unless kscope = class_scope(klass)
+    kscope
   end
+  private :qualified_scope
 
   # Return a hash containing our variables and their values, optionally (and
   # by default) including the values defined in our parent.  Local values
@@ -327,16 +289,6 @@ def parent_module_name
     @parent.source.module_name
   end
 
-  # Return the list of scopes up to the top scope, ordered with our own first.
-  # This is used for looking up variables and defaults.
-  def scope_path
-    if parent
-      [self, parent.scope_path].flatten.compact
-    else
-      [self]
-    end
-  end
-
   # Set defaults for a type.  The typename should already be downcased,
   # so that the syntax is isolated.  We don't do any kind of type-checking
   # here; instead we let the resource do it when the defaults are used.
@@ -347,8 +299,6 @@ def define_settings(type, params)
     params = [params] unless params.is_a?(Array)
 
     params.each { |param|
-      #Puppet.debug "Default for %s is %s => %s" %
-      #    [type,ary[0].inspect,ary[1].inspect]
       if table.include?(param.name)
         raise Puppet::ParseError.new("Default already defined for #{type} { #{param.name} }; cannot redefine", param.line, param.file)
       end
@@ -361,40 +311,42 @@ def define_settings(type, params)
   # to be reassigned.
   #   It's preferred that you use self[]= instead of this; only use this
   # when you need to set options.
-  def setvar(name,value, options = {})
+  def setvar(name, value, options = {})
     table = options[:ephemeral] ? @ephemeral.last : @symtable
     if table.include?(name)
-      unless options[:append]
-        error = Puppet::ParseError.new("Cannot reassign variable #{name}")
-      else
+      if options[:append]
         error = Puppet::ParseError.new("Cannot append, variable #{name} is defined in this scope")
+      else
+        error = Puppet::ParseError.new("Cannot reassign variable #{name}")
       end
       error.file = options[:file] if options[:file]
       error.line = options[:line] if options[:line]
       raise error
     end
 
-    unless options[:append]
+    if options[:append]
+      table[name] = append_value(undef_as('', self[name]), value)
+    else 
       table[name] = value
-    else # append case
-      # lookup the value in the scope if it exists and insert the var
-      table[name] = undef_as('',self[name])
-      # concatenate if string, append if array, nothing for other types
-      case value
-      when Array
-        table[name] += value
-      when Hash
-        raise ArgumentError, "Trying to append to a hash with something which is not a hash is unsupported" unless value.is_a?(Hash)
-        table[name].merge!(value)
-      else
-        table[name] << value
+    end
+  end
+
+  def append_value(bound_value, new_value)
+    case new_value
+    when Array
+      bound_value + new_value
+    when Hash
+      bound_value.merge(new_value)
+    else
+      if bound_value.is_a?(Hash)
+        raise ArgumentError, "Trying to append to a hash with something which is not a hash is unsupported" 
       end
+      bound_value + new_value
     end
   end
+  private :append_value
 
-  # Return the tags associated with this scope.  It's basically
-  # just our parents' tags, plus our type.  We don't cache this value
-  # because our parent tags might change between calls.
+  # Return the tags associated with this scope.
   def tags
     resource.tags
   end
@@ -404,12 +356,6 @@ def to_s
     "Scope(#{@resource})"
   end
 
-  # Undefine a variable; only used for testing.
-  def unsetvar(var)
-    table = ephemeral?(var) ? @ephemeral.last : @symtable
-    table.delete(var) if table.include?(var)
-  end
-
   # remove ephemeral scope up to level
   def unset_ephemeral_var(level=:all)
     if level == :all
diff --git a/spec/integration/parser/scope_spec.rb b/spec/integration/parser/scope_spec.rb
new file mode 100644
index 0000000..6ce31c1
--- /dev/null
+++ b/spec/integration/parser/scope_spec.rb
@@ -0,0 +1,419 @@
+require 'spec_helper'
+require 'puppet_spec/compiler'
+
+describe "Two step scoping for variables" do
+  include PuppetSpec::Compiler
+
+  def expect_the_message_to_be(message, node = Puppet::Node.new('the node')) 
+    catalog = compile_to_catalog(yield, node)
+    catalog.resource('Notify', 'something')[:message].should == message
+  end
+
+  before :each do
+    Puppet.expects(:deprecation_warning).never
+  end
+
+  describe "when using shadowing and inheritance" do
+    it "finds value define in the inherited node" do
+      expect_the_message_to_be('parent_msg') do <<-MANIFEST
+          $var = "top_msg"
+          node parent {
+            $var = "parent_msg"
+          }
+          node default inherits parent {
+            include foo
+          }
+          class foo {
+            notify { 'something': message => $var, }
+          }
+        MANIFEST
+      end
+    end
+
+    it "finds top scope when the class is included before the node defines the var" do
+      expect_the_message_to_be('top_msg') do <<-MANIFEST
+          $var = "top_msg"
+          node parent {
+            include foo
+          }
+          node default inherits parent {
+            $var = "default_msg"
+          }
+          class foo {
+            notify { 'something': message => $var, }
+          }
+        MANIFEST
+      end
+    end
+
+    it "finds top scope when the class is included before the node defines the var" do
+      expect_the_message_to_be('top_msg') do <<-MANIFEST
+          $var = "top_msg"
+          node parent {
+            include foo
+          }
+          node default inherits parent {
+            $var = "default_msg"
+          }
+          class foo {
+            notify { 'something': message => $var, }
+          }
+        MANIFEST
+      end
+    end
+
+
+    it "should find values in its local scope" do
+      expect_the_message_to_be('local_msg') do <<-MANIFEST
+          node default {
+            include baz
+          }
+          class foo {
+          }
+          class bar inherits foo {
+            $var = "local_msg"
+            notify { 'something': message => $var, }
+          }
+          class baz {
+            include bar
+          }
+        MANIFEST
+      end
+    end
+
+    it "should find values in its inherited scope" do
+      expect_the_message_to_be('foo_msg') do <<-MANIFEST
+          node default {
+            include baz
+          }
+          class foo {
+            $var = "foo_msg"
+          }
+          class bar inherits foo {
+            notify { 'something': message => $var, }
+          }
+          class baz {
+            include bar
+          }
+        MANIFEST
+      end
+    end
+
+    it "prefers values in its inherited scope over those in the node (with intermediate inclusion)" do
+      expect_the_message_to_be('foo_msg') do <<-MANIFEST
+          node default {
+            $var = "node_msg"
+            include baz
+          }
+          class foo {
+            $var = "foo_msg"
+          }
+          class bar inherits foo {
+            notify { 'something': message => $var, }
+          }
+          class baz {
+            include bar
+          }
+        MANIFEST
+      end
+    end
+
+    it "prefers values in its inherited scope over those in the node (without intermediate inclusion)" do
+      expect_the_message_to_be('foo_msg') do <<-MANIFEST
+          node default {
+            $var = "node_msg"
+            include bar
+          }
+          class foo {
+            $var = "foo_msg"
+          }
+          class bar inherits foo {
+            notify { 'something': message => $var, }
+          }
+        MANIFEST
+      end
+    end
+
+    it "prefers values in its inherited scope over those from where it is included" do
+      expect_the_message_to_be('foo_msg') do <<-MANIFEST
+          node default {
+            include baz
+          }
+          class foo {
+            $var = "foo_msg"
+          }
+          class bar inherits foo {
+            notify { 'something': message => $var, }
+          }
+          class baz {
+            $var = "baz_msg"
+            include bar
+          }
+        MANIFEST
+      end
+    end
+
+    it "does not used variables from classes included in the inherited scope" do
+      expect_the_message_to_be('node_msg') do <<-MANIFEST
+          node default {
+            $var = "node_msg"
+            include bar
+          }
+          class quux {
+            $var = "quux_msg"
+          }
+          class foo inherits quux {
+          }
+          class baz {
+            include foo
+          }
+          class bar inherits baz {
+            notify { 'something': message => $var, }
+          }
+        MANIFEST
+      end
+    end
+
+    it "does not use a variable from a scope lexically enclosing it" do
+      expect_the_message_to_be('node_msg') do <<-MANIFEST
+          node default {
+            $var = "node_msg"
+            include other::bar
+          }
+          class other {
+            $var = "other_msg"
+            class bar {
+              notify { 'something': message => $var, }
+            }
+          }
+        MANIFEST
+      end
+    end
+
+    it "finds values in its node scope" do
+      expect_the_message_to_be('node_msg') do <<-MANIFEST
+          node default {
+            $var = "node_msg"
+            include baz
+          }
+          class foo {
+          }
+          class bar inherits foo {
+            notify { 'something': message => $var, }
+          }
+          class baz {
+            include bar
+          }
+        MANIFEST
+      end
+    end
+
+    it "finds values in its top scope" do
+      expect_the_message_to_be('top_msg') do <<-MANIFEST
+          $var = "top_msg"
+          node default {
+            include baz
+          }
+          class foo {
+          }
+          class bar inherits foo {
+            notify { 'something': message => $var, }
+          }
+          class baz {
+            include bar
+          }
+        MANIFEST
+      end
+    end
+
+    it "prefers variables from the node over those in the top scope" do
+      expect_the_message_to_be('node_msg') do <<-MANIFEST
+          $var = "top_msg"
+          node default {
+            $var = "node_msg"
+            include foo
+          }
+          class foo {
+            notify { 'something': message => $var, }
+          }
+        MANIFEST
+      end
+    end
+  end
+
+  describe "in situations that used to have dynamic lookup" do
+    it "ignores the dynamic value of the var" do
+      expect_the_message_to_be('node_msg') do <<-MANIFEST
+          node default {
+            $var = "node_msg"
+            include foo
+          }
+          class baz {
+            $var = "baz_msg"
+            include bar
+          }
+          class foo inherits baz {
+          }
+          class bar {
+            notify { 'something': message => $var, }
+          }
+        MANIFEST
+      end
+    end
+
+    it "finds nil when the only set variable is in the dynamic scope" do
+      expect_the_message_to_be(nil) do <<-MANIFEST
+          node default {
+            include baz
+          }
+          class foo {
+          }
+          class bar inherits foo {
+            notify { 'something': message => $var, }
+          }
+          class baz {
+            $var = "baz_msg"
+            include bar
+          }
+        MANIFEST
+      end
+    end
+  end
+
+  describe "using plussignment to change in a new scope" do
+    it "does not change a string in the parent scope" do
+      expect_the_message_to_be('top_msg') do <<-MANIFEST
+          $var = "top_msg"
+          class override {
+            $var += "override"
+            include foo
+          }
+          class foo {
+            notify { 'something': message => $var, }
+          }
+
+          include override
+        MANIFEST
+      end
+    end
+
+    it "does not change an array in the parent scope" do
+      expect_the_message_to_be('top_msg') do <<-MANIFEST
+          $var = ["top_msg"]
+          class override {
+            $var += ["override"]
+            include foo
+          }
+          class foo {
+            notify { 'something': message => $var, }
+          }
+
+          include override
+        MANIFEST
+      end
+    end
+
+    it "concatenates two arrays" do
+      expect_the_message_to_be(['top_msg', 'override']) do <<-MANIFEST
+          $var = ["top_msg"]
+          class override {
+            $var += ["override"]
+            notify { 'something': message => $var, }
+          }
+
+          include override
+        MANIFEST
+      end
+    end
+
+    it "leaves an array of arrays unflattened" do
+      expect_the_message_to_be([['top_msg'], ['override']]) do <<-MANIFEST
+          $var = [["top_msg"]]
+          class override {
+            $var += [["override"]]
+            notify { 'something': message => $var, }
+          }
+
+          include override
+        MANIFEST
+      end
+    end
+
+    it "does not change a hash in the parent scope" do
+      expect_the_message_to_be({"key"=>"top_msg"}) do <<-MANIFEST
+          $var = { "key" => "top_msg" }
+          class override {
+            $var += { "other" => "override" }
+            include foo
+          }
+          class foo {
+            notify { 'something': message => $var, }
+          }
+
+          include override
+        MANIFEST
+      end
+    end
+
+    it "replaces a value of a key in the hash instead of merging the values" do
+      expect_the_message_to_be({"key"=>"override"}) do <<-MANIFEST
+          $var = { "key" => "top_msg" }
+          class override {
+            $var += { "key" => "override" }
+            notify { 'something': message => $var, }
+          }
+
+          include override
+        MANIFEST
+      end
+    end
+  end
+
+  describe "when using an enc" do
+    it "places enc parameters in top scope" do
+      enc_node = Puppet::Node.new("the node", { :parameters => { "var" => 'from_enc' } })
+
+      expect_the_message_to_be('from_enc', enc_node) do <<-MANIFEST
+          notify { 'something': message => $var, }
+        MANIFEST
+      end
+    end
+
+    it "does not allow the enc to specify an existing top scope var" do
+      enc_node = Puppet::Node.new("the_node", { :parameters => { "var" => 'from_enc' } })
+
+      expect { compile_to_catalog("$var = 'top scope'", enc_node) }.should raise_error(Puppet::Error, "Cannot reassign variable var at line 1 on node the_node")
+    end
+
+    it "evaluates enc classes in top scope when there is no node" do
+      enc_node = Puppet::Node.new("the node", { :classes => ['foo'], :parameters => { "var" => 'from_enc' } })
+
+      expect_the_message_to_be('from_enc', enc_node) do <<-MANIFEST
+          class foo {
+            notify { 'something': message => $var, }
+          }
+        MANIFEST
+      end
+    end
+
+    it "evaluates enc classes in the node scope when there is a matching node" do
+      enc_node = Puppet::Node.new("the_node", { :classes => ['foo'] })
+
+      expect_the_message_to_be('from matching node', enc_node) do <<-MANIFEST
+          node inherited {
+            $var = "from inherited"
+          }
+
+          node the_node inherits inherited {
+            $var = "from matching node"
+          }
+
+          class foo {
+            notify { 'something': message => $var, }
+          }
+        MANIFEST
+      end
+    end
+  end
+end
+
diff --git a/spec/lib/puppet_spec/compiler.rb b/spec/lib/puppet_spec/compiler.rb
index 54741c6..949c826 100644
--- a/spec/lib/puppet_spec/compiler.rb
+++ b/spec/lib/puppet_spec/compiler.rb
@@ -1,6 +1,6 @@
 module PuppetSpec::Compiler
-  def compile_to_catalog(string)
+  def compile_to_catalog(string, node = Puppet::Node.new('foonode'))
     Puppet[:code] = string
-    Puppet::Parser::Compiler.compile(Puppet::Node.new('foonode'))
+    Puppet::Parser::Compiler.compile(node)
   end
 end
diff --git a/spec/unit/parser/scope_spec.rb b/spec/unit/parser/scope_spec.rb
index 850b67f..412b7f5 100755
--- a/spec/unit/parser/scope_spec.rb
+++ b/spec/unit/parser/scope_spec.rb
@@ -11,10 +11,6 @@
     @scope.parent = @topscope
   end
 
-  it "should be able to store references to class scopes" do
-    lambda { @scope.class_set "myname", "myscope" }.should_not raise_error
-  end
-
   it "should be able to retrieve class scopes by name" do
     @scope.class_set "myname", "myscope"
     @scope.class_scope("myname").should == "myscope"
@@ -130,6 +126,11 @@
       @scope.should be_include("var")
     end
 
+    it "does not allow changing a set value" do
+      @scope["var"] = "childval"
+      expect { @scope["var"] = "change" }.should raise_error(Puppet::Error, "Cannot reassign variable var")
+    end
+
     it "should be able to detect when variables are not set" do
       @scope.should_not be_include("var")
     end
@@ -146,18 +147,6 @@
       @scope.singleton_class.ancestors.should be_include(Enumerable)
     end
 
-    it "should be able to look up intermediary variables in parent scopes (DEPRECATED)" do
-      Puppet.expects(:deprecation_warning)
-      thirdscope = Puppet::Parser::Scope.new
-      thirdscope.parent = @scope
-      thirdscope.source = Puppet::Resource::Type.new(:hostclass, :foo, :module_name => "foo")
-      @scope.source = Puppet::Resource::Type.new(:hostclass, :bar, :module_name => "bar")
-
-      @topscope.setvar("var2","parentval")
-      @scope.setvar("var2","childval")
-      thirdscope.lookupvar("var2").should == "childval"
-    end
-
     describe "and the variable is qualified" do
       before :each do
         @known_resource_types = @scope.known_resource_types
@@ -235,341 +224,38 @@ def create_class_scope(name)
     end
   end
 
-  describe "when mixing inheritence and inclusion" do
-    include PuppetSpec::Compiler
-
-    def expect_the_message_to_be(message) 
-      catalog = compile_to_catalog(yield)
-      catalog.resource('Notify', 'something')[:message].should == message
-    end
-
-    context "deprecated scoping" do
-      before :each do
-        Puppet.expects(:deprecation_warning).at_least(1)
-      end
-
-      it "prefers values in its included scope over those from the node (DEPRECATED)" do
-        expect_the_message_to_be('baz_msg') do <<-MANIFEST
-            node default {
-              $var = "node_msg"
-              include foo
-            }
-            class baz {
-              $var = "baz_msg"
-              include bar
-            }
-            class foo inherits baz {
-            }
-            class bar {
-              notify { 'something': message => $var, }
-            }
-          MANIFEST
-        end
-      end
-
-      it "finds values in its included scope (DEPRECATED)" do
-        expect_the_message_to_be('baz_msg') do <<-MANIFEST
-            node default {
-              include baz
-            }
-            class foo {
-            }
-            class bar inherits foo {
-              notify { 'something': message => $var, }
-            }
-            class baz {
-              $var = "baz_msg"
-              include bar
-            }
-          MANIFEST
-        end
-      end
-
-      it "recognizes a dynamically scoped boolean (DEPRECATED)" do
-        expect_the_message_to_be(true) do <<-MANIFEST
-            node default {
-              $var = false
-              include baz
-            }
-            class foo {
-            }
-            class bar inherits foo {
-              notify { 'something': message => $var, }
-            }
-            class baz {
-              $var = true
-              include bar
-            }
-          MANIFEST
-        end
-      end
-    end
-
-    context "supported scoping" do
-      before :each do
-        Puppet.expects(:deprecation_warning).never
-      end
-
-      it "finds value define in the inherited node" do
-        expect_the_message_to_be('parent_msg') do <<-MANIFEST
-            $var = "top_msg"
-            node parent {
-              $var = "parent_msg"
-            }
-            node default inherits parent {
-              include foo
-            }
-            class foo {
-              notify { 'something': message => $var, }
-            }
-          MANIFEST
-        end
-      end
-
-      it "finds top scope when the class is included before the node defines the var" do
-        expect_the_message_to_be('top_msg') do <<-MANIFEST
-            $var = "top_msg"
-            node parent {
-              include foo
-            }
-            node default inherits parent {
-              $var = "default_msg"
-            }
-            class foo {
-              notify { 'something': message => $var, }
-            }
-          MANIFEST
-        end
-      end
-
-      it "finds top scope when the class is included before the node defines the var" do
-        expect_the_message_to_be('top_msg') do <<-MANIFEST
-            $var = "top_msg"
-            node parent {
-              include foo
-            }
-            node default inherits parent {
-              $var = "default_msg"
-            }
-            class foo {
-              notify { 'something': message => $var, }
-            }
-          MANIFEST
-        end
-      end
-
-
-      it "should find values in its local scope" do
-        expect_the_message_to_be('local_msg') do <<-MANIFEST
-            node default {
-              include baz
-            }
-            class foo {
-            }
-            class bar inherits foo {
-              $var = "local_msg"
-              notify { 'something': message => $var, }
-            }
-            class baz {
-              include bar
-            }
-          MANIFEST
-        end
-      end
-
-      it "should find values in its inherited scope" do
-        expect_the_message_to_be('foo_msg') do <<-MANIFEST
-            node default {
-              include baz
-            }
-            class foo {
-              $var = "foo_msg"
-            }
-            class bar inherits foo {
-              notify { 'something': message => $var, }
-            }
-            class baz {
-              include bar
-            }
-          MANIFEST
-        end
-      end
-
-      it "prefers values in its inherited scope over those in the node (with intermediate inclusion)" do
-        expect_the_message_to_be('foo_msg') do <<-MANIFEST
-            node default {
-              $var = "node_msg"
-              include baz
-            }
-            class foo {
-              $var = "foo_msg"
-            }
-            class bar inherits foo {
-              notify { 'something': message => $var, }
-            }
-            class baz {
-              include bar
-            }
-          MANIFEST
-        end
-      end
-
-      it "prefers values in its inherited scope over those in the node (without intermediate inclusion)" do
-        expect_the_message_to_be('foo_msg') do <<-MANIFEST
-            node default {
-              $var = "node_msg"
-              include bar
-            }
-            class foo {
-              $var = "foo_msg"
-            }
-            class bar inherits foo {
-              notify { 'something': message => $var, }
-            }
-          MANIFEST
-        end
-      end
-
-      it "prefers values in its inherited scope over those from where it is included" do
-        expect_the_message_to_be('foo_msg') do <<-MANIFEST
-            node default {
-              include baz
-            }
-            class foo {
-              $var = "foo_msg"
-            }
-            class bar inherits foo {
-              notify { 'something': message => $var, }
-            }
-            class baz {
-              $var = "baz_msg"
-              include bar
-            }
-          MANIFEST
-        end
-      end
-
-      it "does not used variables from classes included in the inherited scope" do
-        expect_the_message_to_be('node_msg') do <<-MANIFEST
-            node default {
-              $var = "node_msg"
-              include bar
-            }
-            class quux {
-              $var = "quux_msg"
-            }
-            class foo inherits quux {
-            }
-            class baz {
-              include foo
-            }
-            class bar inherits baz {
-              notify { 'something': message => $var, }
-            }
-          MANIFEST
-        end
-      end
-
-      it "does not use a variable from a scope lexically enclosing it" do
-        expect_the_message_to_be('node_msg') do <<-MANIFEST
-            node default {
-              $var = "node_msg"
-              include other::bar
-            }
-            class other {
-              $var = "other_msg"
-              class bar {
-                notify { 'something': message => $var, }
-              }
-            }
-          MANIFEST
-        end
-      end
-
-      it "finds values in its node scope" do
-        expect_the_message_to_be('node_msg') do <<-MANIFEST
-            node default {
-              $var = "node_msg"
-              include baz
-            }
-            class foo {
-            }
-            class bar inherits foo {
-              notify { 'something': message => $var, }
-            }
-            class baz {
-              include bar
-            }
-          MANIFEST
-        end
-      end
-
-      it "finds values in its top scope" do
-        expect_the_message_to_be('top_msg') do <<-MANIFEST
-            $var = "top_msg"
-            node default {
-              include baz
-            }
-            class foo {
-            }
-            class bar inherits foo {
-              notify { 'something': message => $var, }
-            }
-            class baz {
-              include bar
-            }
-          MANIFEST
-        end
-      end
-
-      it "prefers variables from the node over those in the top scope" do
-        expect_the_message_to_be('node_msg') do <<-MANIFEST
-            $var = "top_msg"
-            node default {
-              $var = "node_msg"
-              include foo
-            }
-            class foo {
-              notify { 'something': message => $var, }
-            }
-          MANIFEST
-        end
-      end
-    end
-  end
-
   describe "when variables are set with append=true" do
     it "should raise error if the variable is already defined in this scope" do
-      @scope.setvar("var","1", :append => false)
-      lambda { @scope.setvar("var","1", :append => true) }.should raise_error(Puppet::ParseError)
+      @scope.setvar("var", "1", :append => false)
+      expect { @scope.setvar("var", "1", :append => true) }.should raise_error(Puppet::ParseError, "Cannot append, variable var is defined in this scope")
     end
 
     it "should lookup current variable value" do
       @scope.expects(:[]).with("var").returns("2")
-      @scope.setvar("var","1", :append => true)
+      @scope.setvar("var", "1", :append => true)
     end
 
     it "should store the concatenated string '42'" do
-      @topscope.setvar("var","4", :append => false)
-      @scope.setvar("var","2", :append => true)
+      @topscope.setvar("var", "4", :append => false)
+      @scope.setvar("var", "2", :append => true)
       @scope["var"].should == "42"
     end
 
     it "should store the concatenated array [4,2]" do
-      @topscope.setvar("var",[4], :append => false)
-      @scope.setvar("var",[2], :append => true)
+      @topscope.setvar("var", [4], :append => false)
+      @scope.setvar("var", [2], :append => true)
       @scope["var"].should == [4,2]
     end
 
     it "should store the merged hash {a => b, c => d}" do
-      @topscope.setvar("var",{"a" => "b"}, :append => false)
-      @scope.setvar("var",{"c" => "d"}, :append => true)
+      @topscope.setvar("var", {"a" => "b"}, :append => false)
+      @scope.setvar("var", {"c" => "d"}, :append => true)
       @scope["var"].should == {"a" => "b", "c" => "d"}
     end
 
     it "should raise an error when appending a hash with something other than another hash" do
-      @topscope.setvar("var",{"a" => "b"}, :append => false)
-      lambda { @scope.setvar("var","not a hash", :append => true) }.should raise_error
+      @topscope.setvar("var", {"a" => "b"}, :append => false)
+      expect { @scope.setvar("var", "not a hash", :append => true) }.should raise_error(ArgumentError, "Trying to append to a hash with something which is not a hash is unsupported")
     end
   end
 
@@ -663,7 +349,7 @@ class foo {
 
     it "should raise an error when setting it again" do
       @scope.setvar("1", :value2, :ephemeral => true)
-      lambda { @scope.setvar("1", :value3, :ephemeral => true) }.should raise_error
+      expect { @scope.setvar("1", :value3, :ephemeral => true) }.should raise_error
     end
 
     it "should declare ephemeral number only variable names" do
@@ -750,7 +436,7 @@ class foo {
     end
 
     it "should accept only MatchData" do
-      lambda { @scope.ephemeral_from("match") }.should raise_error
+      expect { @scope.ephemeral_from("match") }.should raise_error
     end
 
     it "should set $0 with the full match" do
@@ -773,27 +459,6 @@ class foo {
     end
   end
 
-  describe "when unsetting variables" do
-    it "should be able to unset normal variables" do
-      @scope["foo"] = "bar"
-      @scope.unsetvar("foo")
-      @scope["foo"].should be_nil
-    end
-
-    it "should be able to unset ephemeral variables" do
-      @scope.setvar("0", "bar", :ephemeral => true)
-      @scope.unsetvar("0")
-      @scope["0"].should be_nil
-    end
-
-    it "should not unset ephemeral variables in previous ephemeral scope" do
-      @scope.setvar("0", "bar", :ephemeral => true)
-      @scope.new_ephemeral
-      @scope.unsetvar("0")
-      @scope["0"].should == "bar"
-    end
-  end
-
   it "should use its namespaces to find hostclasses" do
     klass = @scope.known_resource_types.add Puppet::Resource::Type.new(:hostclass, "a::b::c")
     @scope.add_namespace "a::b"
@@ -816,7 +481,7 @@ class foo {
     it "should fail if a default is already defined and a new default is being defined" do
       param = Puppet::Parser::Resource::Param.new(:name => :myparam, :value => "myvalue", :source => stub("source"))
       @scope.define_settings(:mytype, param)
-      lambda { @scope.define_settings(:mytype, param) }.should raise_error(Puppet::ParseError)
+      expect { @scope.define_settings(:mytype, param) }.should raise_error(Puppet::ParseError)
     end
 
     it "should return multiple defaults at once" do

    

--
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