(PUP-3351) Test using example of problematic behavior

After talking with Reid Vandewiele, it turns out that the problematic
behavior isn't directly the order of the classes themselves. Previous
assertions that there are possible conflicts from the classes when they
were done in one order or another are incorrect. The classes, when any
have parameters, are kept by the node in a hash keyed off of the class
name, which means that there is no possibility of them creating a
duplicate resource error.

The actual usecase comes down to *how* parameterized classes vs
unparameterized classes are often used. A parameterized class can often
be simply a set of shared data that the unparameterized classes depend
on (or inherit from) parameterized classes.

This modifies the test case for the reordering change to provide an
example of the use case here.

2 files changed, 19 insertions, 33 deletions

diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index 43eb533f8..125b8e9f2 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -187,8 +187,6 @@ class Puppet::Parser::Compiler
       classes_without_params = @node.classes
     end
 
-    # (PUP-3351) Avoid "Duplicate declaration" errors by evaluating classes
-    # with params prior to classes without params.
     evaluate_classes(classes_with_params, @node_scope || topscope)
     evaluate_classes(classes_without_params, @node_scope || topscope)
   end
diff --git a/spec/unit/parser/compiler_spec.rb b/spec/unit/parser/compiler_spec.rb
index 1d6a25277..74cb326d9 100755
--- a/spec/unit/parser/compiler_spec.rb
+++ b/spec/unit/parser/compiler_spec.rb
@@ -1,6 +1,6 @@
-#! /usr/bin/env ruby
 require 'spec_helper'
 require 'puppet_spec/compiler'
+require 'matchers/resource'
 
 class CompilerTestResource
   attr_accessor :builtin, :virtual, :evaluated, :type, :title
@@ -53,6 +53,7 @@ end
 
 describe Puppet::Parser::Compiler do
   include PuppetSpec::Files
+  include Matchers::Resource
 
   def resource(type, title)
     Puppet::Parser::Resource.new(type, title, :scope => @scope)
@@ -422,36 +423,6 @@ describe Puppet::Parser::Compiler do
       @compiler.catalog.should be_vertex(resource)
     end
 
-    context 'and evaluating classes declared from an ENC' do
-      # Expected sequence of class evaluation
-      let(:seq) { sequence('partitioned_node_classes') }
-      # class { [c1, c0]: p1 => v1 }
-      let(:param_classes) do
-        { 'c1' => { 'p1' => 'v1' }, 'c0' => { 'p1' => 'v1' } }
-      end
-      # include 'c2'
-      let(:plain_classes) { { 'c2' => {} } }
-
-      let(:classes) { param_classes.merge(plain_classes) }
-
-      before :each do
-        # Stub _except_ evaluate_node_classes
-        compile_stub(:evaluate_node_classes)
-      end
-
-      it 'then evaluates classes declared with parameters before plain classes' do
-        @node.stubs(:classes).returns(classes)
-
-        [param_classes, plain_classes.keys].each do |partition|
-          @compiler.expects(:evaluate_classes).
-            with(partition, @compiler.topscope).
-            in_sequence(seq)
-        end
-
-        @compiler.compile
-      end
-    end
-
     it "should fail to add resources that conflict with existing resources" do
       path = make_absolute("/foo")
       file1 = resource(:file, path)
@@ -894,6 +865,23 @@ describe Puppet::Parser::Compiler do
         it "should fail if the class doesn't exist" do
           expect { compile_to_catalog('', node) }.to raise_error(Puppet::Error, /Could not find class something/)
         end
+
+        it 'evaluates classes declared with parameters before unparameterized classes' do
+          node = Puppet::Node.new('someone', :classes => { 'app::web' => {}, 'app' => { 'port' => 8080 } })
+          manifest = <<-MANIFEST
+          class app($port = 80) { }
+
+          class app::web($port = $app::port) inherits app {
+            notify { expected: message => "$port" }
+          }
+          MANIFEST
+
+          catalog = compile_to_catalog(manifest, node)
+
+          expect(catalog).to have_resource("Class[App]").with_parameter(:port, 8080)
+          expect(catalog).to have_resource("Class[App::Web]")
+          expect(catalog).to have_resource("Notify[expected]").with_parameter(:message, "8080")
+        end
       end
     end
   end