(PUP-11689) Turn on strict_variables to enable strict mode by default

This commit sets strict_variables to true and modifies any tests that fail due to this change. To completely enable strict mode by default, strict will also need to be set to error instead of warning. This work will be done in PUP-11725.

Author: AriaXLi

lib/puppet/defaults.rb

@@ -2210,7 +2210,7 @@ def self.initialize_default_settings!(settings)
     EOT
     },
   :strict_variables => {
-    :default => false,
+    :default => true,
     :type => :boolean,
     :desc => <<-'EOT'
       Causes an evaluation error when referencing unknown variables. (This does not affect

spec/integration/application/agent_spec.rb

@@ -993,13 +993,11 @@ def mounts
         agent.command_line.args << '--test'
         expect {
           agent.run
-        }.to exit_with(2)
+        }.to exit_with(1)
           .and output(
-            match(/defined 'message' as 'legacy '/)
-              .and match(/defined 'message' as 'custom a custom value'/)
-              .and match(/defined 'message' as 'external an external value'/)
-            ).to_stdout
-          .and output(/Warning: Unknown variable: 'osfamily'/).to_stderr
+            match(/Error: Evaluation Error: Unknown variable: 'osfamily'/)
+              .and match(/Error: Could not retrieve catalog from remote server: Error 500 on SERVER:/)
+          ).to_stderr
       end
     end
   end

spec/integration/parser/compiler_spec.rb

@@ -693,8 +693,19 @@ class a { $_a = 10}
         end.to raise_error(/Illegal variable name/)
       end
 
-      it 'a missing variable as default value becomes undef' do
-        # strict variables not on
+      it 'a missing variable as default causes an evaluation error' do
+        # when strict variables not on
+        expect {
+          compile_to_catalog(<<-MANIFEST)
+          class a ($b=$x) { notify {test: message=>"yes ${undef == $b}" } }
+            include a
+          MANIFEST
+        }.to raise_error(/Evaluation Error: Unknown variable: 'x'/)
+      end
+
+      it 'a missing variable as default value becomes undef when strict_variables is off' do
+        # strict variables on by default for 8.x
+        Puppet[:strict_variables] = false
         catalog = compile_to_catalog(<<-MANIFEST)
         class a ($b=$x) { notify {test: message=>"yes ${undef == $b}" } }
           include a

spec/integration/parser/conditionals_spec.rb

@@ -68,6 +68,8 @@
     end
 
     it "evaluates undefined variables as false" do
+      # strict_variables is off so behavior this test is trying to check isn't stubbed out
+      Puppet[:strict_variables] = false
       catalog = compile_to_catalog(<<-CODE)
       if $undef_var {
       } else {

spec/integration/parser/scope_spec.rb

@@ -276,6 +276,8 @@ class a inherits parent { }
 
     ['a:.b', '::a::b'].each do |ref|
       it "does not resolve a qualified name on the form #{ref} against top scope" do
+        # strict_variables is off so behavior this test is trying to check isn't stubbed out
+        Puppet[:strict_variables] = false
         expect_the_message_not_to_be("from topscope") do <<-"MANIFEST"
               class c {
                 notify { 'something': message => "$#{ref}" }
@@ -297,6 +299,8 @@ class a inherits parent { }
 
     ['a:.b', '::a::b'].each do |ref|
       it "does not resolve a qualified name on the form #{ref} against node scope" do
+        # strict_variables is off so behavior this test is trying to check isn't stubbed out
+        Puppet[:strict_variables] = false
         expect_the_message_not_to_be("from node") do <<-MANIFEST
               class c {
                 notify { 'something': message => "$a::b" }
@@ -626,8 +630,9 @@ class bar {
       end
     end
 
-    it "finds nil when the only set variable is in the dynamic scope" do
-      expect_the_message_to_be(nil) do <<-MANIFEST
+    it "raises an evaluation error when the only set variable is in the dynamic scope" do
+      expect {
+        compile_to_catalog(<<-MANIFEST)
             node default {
               include baz
             }
@@ -641,7 +646,7 @@ class baz {
               include bar
             }
         MANIFEST
-      end
+      }.to raise_error(/Evaluation Error: Unknown variable: 'var'./)
     end
 
     it "ignores the value in the dynamic scope for a defined type" do

spec/unit/application/lookup_spec.rb

@@ -673,6 +673,8 @@ def run_lookup(lookup)
       let(:node) { Puppet::Node.new("testnode", :facts => facts, :environment => 'puppet_func_provider') }
 
       it "works OK in the absense of '--compile'" do
+        # strict_variables is off so behavior this test is trying to check isn't stubbed out
+        Puppet[:strict_variables] = false
         lookup.options[:node] = node
         allow(lookup.command_line).to receive(:args).and_return(['c'])
         lookup.options[:render_as] = :s

spec/unit/functions/epp_spec.rb

@@ -18,7 +18,12 @@
       expect(eval_template("all your base <%= $what::is %> to us")).to eq("all your base are belong to us")
     end
 
-    it "get nil accessing a variable that does not exist" do
+    it "gets error accessing a variable that does not exist" do
+      expect { eval_template("<%= $kryptonite == undef %>")}.to raise_error(/Evaluation Error: Unknown variable: 'kryptonite'./)
+    end
+
+    it "get nil accessing a variable that does not exist when strict_variables is off" do
+      Puppet[:strict_variables] = false
       expect(eval_template("<%= $kryptonite == undef %>")).to eq("true")
     end
 

spec/unit/functions/inline_epp_spec.rb

@@ -16,7 +16,12 @@
       expect(eval_template("all your base <%= $what %> to us")).to eq("all your base are belong to us")
     end
 
-    it "get nil accessing a variable that does not exist" do
+    it "gets error accessing a variable that does not exist" do
+      expect { eval_template("<%= $kryptonite == undef %>")}.to raise_error(/Evaluation Error: Unknown variable: 'kryptonite'./)
+    end
+
+    it "get nil accessing a variable that does not exist when strict_variables is off" do
+      Puppet[:strict_variables] = false
       expect(eval_template("<%= $kryptonite == undef %>")).to eq("true")
     end
 

spec/unit/functions/return_spec.rb

@@ -19,6 +19,8 @@
     end
 
     it 'with undef value as function result when not given an argument' do
+      # strict_variables is off so behavior this test is trying to check isn't stubbed out
+      Puppet[:strict_variables] = false
       expect(compile_to_catalog(<<-CODE)).to have_resource('Notify[xy]')
           function please_return() {
             [1,2,3].map |$x| { if $x == 1 { return() } 200 }

spec/unit/indirector/catalog/compiler_spec.rb

@@ -514,9 +514,10 @@ def a_request_that_contains(facts)
 
     it "should not add 'pe_serverversion' when FOSS" do
       allow(Puppet::Node.indirection).to receive(:find).with(node_name, anything).and_return(node)
-      catalog = compiler.find(request)
-
-      expect(catalog.resource('notify', 'PE Version: 2019.2.0')).to be_nil
+      expect {
+        catalog = compiler.find(request)
+        catalog.resource('notify', 'PE Version: 2019.2.0') 
+      }.to raise_error(/Evaluation Error: Unknown variable: 'pe_serverversion'./)
     end
 
     it "should add 'pe_serverversion' when PE" do