Fixing #800 by refactoring how configurations are retrieved

from the server.  The real problem was getting all of the validation
done before any caching, which required a good bit more refactoring
than I expected.

In actuality, this commit is relatively small even though it covers
many files; most of the changes just make the code clearer or shorter.

2 files changed, 23 insertions, 117 deletions

diff --git a/test/network/handler/master.rb b/test/network/handler/master.rb
index 694888f4d..25117030e 100755
--- a/test/network/handler/master.rb
+++ b/test/network/handler/master.rb
@@ -13,39 +13,11 @@ class TestMaster < Test::Unit::TestCase
         Puppet::Indirector::Indirection.clear_cache
     end
 
-    def test_defaultmanifest
-        textfiles { |file|
-            Puppet[:manifest] = file
-            client = nil
-            master = nil
-            assert_nothing_raised() {
-                # this is the default server setup
-                master = Puppet::Network::Handler.master.new(
-                    :Manifest => file,
-                    :UseNodes => false,
-                    :Local => true
-                )
-            }
-            assert_nothing_raised() {
-                client = Puppet::Network::Client.master.new(
-                    :Master => master
-                )
-            }
-
-            # pull our configuration
-            assert_nothing_raised() {
-                client.getconfig
-                stopservices
-                Puppet::Type.allclear
-            }
-
-            break
-        }
-    end
-
+    # Make sure that files are reread when they change.
     def test_filereread
         # Start with a normal setting
         Puppet[:filetimeout] = 15
+
         manifest = mktestmanifest()
 
         facts = Puppet::Network::Client.master.facts
@@ -63,35 +35,12 @@ class TestMaster < Test::Unit::TestCase
                 :Local => true
             )
         }
-        assert_nothing_raised() {
-            client = Puppet::Network::Client.master.new(
-                :Master => master
-            )
-        }
 
-        assert(client, "did not create master client")
-        # The client doesn't have a config, so it can't be up to date
-        assert(! client.fresh?(facts),
-            "Client is incorrectly up to date")
-
-        Puppet.settings.use(:main)
-        config = nil
-        assert_nothing_raised {
-            config = client.getconfig
-            config.apply
-        }
-
-        # Now it should be up to date
-        assert(client.fresh?(facts), "Client is not up to date")
+        config = master.getconfig({"hostname" => "blah"})
 
         # Cache this value for later
         parse1 = master.freshness("mynode")
 
-        # Verify the config got applied
-        assert(FileTest.exists?(@createdfile),
-            "Created file %s does not exist" % @createdfile)
-        Puppet::Type.allclear
-
         sleep 1.5
         # Create a new manifest
         File.open(manifest, "w") { |f|
@@ -101,7 +50,6 @@ class TestMaster < Test::Unit::TestCase
         # Verify that the master doesn't immediately reparse the file; we
         # want to wait through the timeout
         assert_equal(parse1, master.freshness("mynode"), "Master did not wait through timeout")
-        assert(client.fresh?(facts), "Client is not up to date")
 
         # Then eliminate it
         Puppet[:filetimeout] = 0
@@ -109,16 +57,8 @@ class TestMaster < Test::Unit::TestCase
         # Now make sure the master does reparse
         #Puppet.notice "%s vs %s" % [parse1, master.freshness]
         assert(parse1 != master.freshness("mynode"), "Master did not reparse file")
-        assert(! client.fresh?(facts), "Client is incorrectly up to date")
-
-        # Retrieve and apply the new config
-        assert_nothing_raised {
-            config = client.getconfig
-            config.apply
-        }
-        assert(client.fresh?(facts), "Client is not up to date")
 
-        assert(FileTest.exists?(file2), "Second file %s does not exist" % file2)
+        assert(master.getconfig({"hostname" => "blah"}) != config, "Did not use reloaded config")
     end
 
     # Make sure we're correctly doing clientname manipulations.
diff --git a/test/network/handler/runner.rb b/test/network/handler/runner.rb
index 402b27ffc..171458ffa 100755
--- a/test/network/handler/runner.rb
+++ b/test/network/handler/runner.rb
@@ -29,7 +29,8 @@ class TestHandlerRunner < Test::Unit::TestCase
         client
     end
 
-    def test_runner
+    def setup
+        super
         FileUtils.mkdir_p(Puppet[:statedir])
         Puppet[:ignoreschedules] = false
         # Okay, make our manifest
@@ -37,7 +38,7 @@ class TestHandlerRunner < Test::Unit::TestCase
         created = tempfile()
         # We specify the schedule here, because I was having problems with
         # using default schedules.
-        code = %{
+        @code = %{
                 class yayness {
                     schedule { "yayness": period => weekly }
                     file { "#{created}": ensure => file, schedule => yayness }
@@ -46,59 +47,24 @@ class TestHandlerRunner < Test::Unit::TestCase
                 include yayness
             }
 
-        client = mkclient(code)
+        @client = mkclient(@code)
 
-        runner = nil
-        assert_nothing_raised {
-            runner = Puppet::Network::Handler.runner.new
-        }
-        # First: tags
-        # Second: ignore schedules true/false
-        # Third: background true/false
-        # Fourth: whether file should exist true/false
-        [
-            ["with no backgrounding",
-                nil, true, true, true],
-            ["in the background",
-                nil, true, false, true],
-            ["with a bad tag",
-                ["coolness"], true, false, false],
-            ["with another bad tag",
-                "coolness", true, false, false],
-            ["with a good tag",
-                ["coolness", "yayness"], true, false, true],
-            ["with another good tag",
-                ["yayness"], true, false, true],
-            ["with a third good tag",
-                "yayness", true, false, true],
-            ["with no tags",
-                "", true, false, true],
-            ["not ignoring schedules",
-                nil, false, false, false],
-            ["ignoring schedules",
-                nil, true, false, true],
-        ].each do |msg, tags, ignore, fg, shouldexist|
-            if FileTest.exists?(created)
-                File.unlink(created)
-            end
-            assert_nothing_raised {
-                # Try it without backgrounding
-                runner.run(tags, ignore, fg)
-            }
+        @runner = Puppet::Network::Handler.runner.new
+    end
+
+    def test_runner_when_in_foreground
+        @client.expects(:run).with(:tags => "mytags", :ignoreschedules => true)
+
+        Process.expects(:newthread).never
 
-            unless fg
-                Puppet.join
-            end
-
-            if shouldexist
-                assert(FileTest.exists?(created), "File did not get created " +
-                    msg)
-            else
-                assert(!FileTest.exists?(created), "File got created incorrectly " +
-                    msg)
-            end
-        end
+        @runner.run("mytags", true, true)
     end
-end
 
+    def test_runner_when_in_background
+        @client.expects(:run).with(:tags => "mytags", :ignoreschedules => true)
+
+        Puppet.expects(:newthread).yields
 
+        @runner.run("mytags", true, false)
+    end
+end