From ecb873d3a6d2eabc931457bd8f4c185c990976db Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Thu, 13 Mar 2008 12:36:21 -0500
Subject: Fixing #1118 -- downloading plugins and facts now ignores noop. Note
 that this changes the behaviour a bit -- the resource's noop setting always
 beats the global setting (previously, whichever was true would win).

---
 CHANGELOG                            |  5 +++++
 lib/puppet/metatype/evaluation.rb    |  6 +++++-
 lib/puppet/network/client/master.rb  | 14 ++-----------
 lib/puppet/type.rb                   |  2 --
 spec/unit/ral/type/noop_metaparam.rb | 38 ++++++++++++++++++++++++++++++++++++
 test/ral/manager/type.rb             | 16 ---------------
 6 files changed, 50 insertions(+), 31 deletions(-)
 create mode 100755 spec/unit/ral/type/noop_metaparam.rb

diff --git a/CHANGELOG b/CHANGELOG
index a710e20a2..25ecd55d2 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,8 @@
+    Fixing #1118 -- downloading plugins and facts now ignores noop.
+    Note that this changes the behaviour a bit -- the resource's
+    noop setting always beats the global setting (previously,
+    whichever was true would win).
+
     The change in checksums from 'timestamp' to 'mtime' no longer
     result in updates on every run (#1116).
 
diff --git a/lib/puppet/metatype/evaluation.rb b/lib/puppet/metatype/evaluation.rb
index 08756e988..36328c537 100644
--- a/lib/puppet/metatype/evaluation.rb
+++ b/lib/puppet/metatype/evaluation.rb
@@ -108,7 +108,11 @@ class Puppet::Type
 
     # Are we running in noop mode?
     def noop?
-        @noop || Puppet[:noop]
+        if defined?(@noop)
+            @noop
+        else
+            Puppet[:noop]
+        end
     end
 
     def noop
diff --git a/lib/puppet/network/client/master.rb b/lib/puppet/network/client/master.rb
index 913c51b3d..e914d5c69 100644
--- a/lib/puppet/network/client/master.rb
+++ b/lib/puppet/network/client/master.rb
@@ -320,7 +320,8 @@ class Puppet::Network::Client::Master < Puppet::Network::Client
             :group => Process.gid,
             :purge => true,
             :force => true,
-            :backup => false
+            :backup => false,
+            :noop => false
         }
 
         if args[:ignore]
@@ -331,9 +332,6 @@ class Puppet::Network::Client::Master < Puppet::Network::Client
         
         Puppet.info "Retrieving #{args[:name]}s"
 
-        noop = Puppet[:noop]
-        Puppet[:noop] = false
-
         files = []
         begin
             Timeout::timeout(self.timeout) do
@@ -355,14 +353,6 @@ class Puppet::Network::Client::Master < Puppet::Network::Client
         downconfig.clear
 
         return files
-    ensure
-        # I can't imagine why this is necessary, but apparently at last one person has had problems with noop
-        # being nil here.
-        if noop.nil?
-            Puppet[:noop] = false
-        else
-            Puppet[:noop] = noop
-        end
     end
 
     # Retrieve facts from the central server.
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 09003e8f5..f8949ec90 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -155,8 +155,6 @@ class Type
             @parameters = {}
         end
 
-        # set defalts
-        @noop = false
         # keeping stats for the total number of changes, and how many were
         # completely sync'ed
         # this isn't really sufficient either, because it adds lots of special
diff --git a/spec/unit/ral/type/noop_metaparam.rb b/spec/unit/ral/type/noop_metaparam.rb
new file mode 100755
index 000000000..0cbed3714
--- /dev/null
+++ b/spec/unit/ral/type/noop_metaparam.rb
@@ -0,0 +1,38 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../../spec_helper'
+
+require 'puppet/metatype/metaparams'
+
+describe Puppet::Type.type(:file).attrclass(:noop) do
+    before do
+        @file = Puppet::Type.newfile :path => "/what/ever"
+    end
+
+    after { Puppet::Type::File.clear }
+
+    it "should accept true as a value" do
+        lambda { @file[:noop] = true }.should_not raise_error
+    end
+
+    it "should accept false as a value" do
+        lambda { @file[:noop] = false }.should_not raise_error
+    end
+
+    describe "when set on a resource" do
+        it "should default to the :noop setting" do
+            Puppet.settings.expects(:value).with(:noop).returns "myval"
+            @file.noop.should == "myval"
+        end
+
+        it "should prefer true values from the attribute" do
+            @file[:noop] = true
+            @file.noop.should be_true
+        end
+
+        it "should prefer false values from the attribute" do
+            @file[:noop] = false
+            @file.noop.should be_false
+        end
+    end
+end
diff --git a/test/ral/manager/type.rb b/test/ral/manager/type.rb
index 324550acb..c6efe4f00 100755
--- a/test/ral/manager/type.rb
+++ b/test/ral/manager/type.rb
@@ -678,22 +678,6 @@ class TestType < Test::Unit::TestCase
         assert_equal("Exec[yay]", exec.ref)
     end
     
-    def test_noop_metaparam
-        file = Puppet::Type.newfile :path => tempfile
-        assert(!file.noop, "file incorrectly in noop")
-        
-        assert_nothing_raised do
-            file[:noop] = true
-        end
-        assert(file.noop, "file should be in noop")
-        
-        # Now set the main one
-        Puppet[:noop] = true
-        assert(file.noop, "file should be in noop")
-        file[:noop] = false
-        assert(file.noop, "file should be in noop")
-    end
-    
     def test_path
         config = mk_catalog
 
-- 
cgit