factor start out of runner.py

This punts on the tests.  For the record, I tried, and madness ensued.

master/buildbot/scripts/restart.py

@@ -13,7 +13,7 @@
 #
 # Copyright Buildbot Team Members
 
-from buildbot.scripts import base, stop, startup
+from buildbot.scripts import base, stop, start
 
 def restart(config):
     basedir = config['basedir']
@@ -27,4 +27,4 @@ def restart(config):
         return 1
     if not quiet:
         print "now restarting buildbot process.."
-    return startup.start(config)
+    return start.start(config)

master/buildbot/scripts/runner.py

@@ -847,13 +847,8 @@ def run():
         from buildbot.scripts import upgrade_master
         sys.exit(upgrade_master.upgradeMaster(so))
     elif command == "start":
-        from buildbot.scripts.startup import start
-
-        if not base.isBuildmasterDir(so['basedir']):
-            print "not a buildmaster directory"
-            sys.exit(1)
-
-        start(so)
+        from buildbot.scripts.start import start
+        sys.exit(start(so))
     elif command == "stop":
         from buildbot.scripts import stop
         sys.exit(stop.stop(so, wait=True))

master/buildbot/scripts/startup.py → master/buildbot/scripts/start.py

@@ -15,29 +15,29 @@
 
 
 import os, sys, time
+from buildbot.scripts import base
+from twisted.internet import reactor
+from twisted.python.runtime import platformType
+from buildbot.scripts.logwatcher import LogWatcher
+from buildbot.scripts.logwatcher import BuildmasterTimeoutError
+from buildbot.scripts.logwatcher import ReconfigError
 
 class Follower:
-    def follow(self):
-        from twisted.internet import reactor
-        from buildbot.scripts.logwatcher import LogWatcher
+    def follow(self, basedir):
         self.rc = 0
         print "Following twistd.log until startup finished.."
-        lw = LogWatcher("twistd.log")
+        lw = LogWatcher(os.path.join(basedir, "twistd.log"))
         d = lw.start()
         d.addCallbacks(self._success, self._failure)
         reactor.run()
         return self.rc
 
     def _success(self, _):
-        from twisted.internet import reactor
         print "The buildmaster appears to have (re)started correctly."
         self.rc = 0
         reactor.stop()
 
     def _failure(self, why):
-        from twisted.internet import reactor
-        from buildbot.scripts.logwatcher import BuildmasterTimeoutError
-        from buildbot.scripts.logwatcher import ReconfigError
         if why.check(BuildmasterTimeoutError):
             print """
 The buildmaster took more than 10 seconds to start, so we were unable to
@@ -61,33 +61,33 @@ def _failure(self, why):
 
 
 def start(config):
-    os.chdir(config['basedir'])
-    if not os.path.exists("buildbot.tac"):
-        print "This doesn't look like a buildbot base directory:"
-        print "No buildbot.tac file."
-        print "Giving up!"
-        sys.exit(1)
+    if not base.isBuildmasterDir(config['basedir']):
+        print "not a buildmaster directory"
+        return 1
+
     if config['quiet']:
-        return launch(config)
+        launch(config)
+        return 0
 
     # we probably can't do this os.fork under windows
-    from twisted.python.runtime import platformType
     if platformType == "win32":
-        return launch(config)
+        launch(config)
+        return 0
 
     # fork a child to launch the daemon, while the parent process tails the
     # logfile
     if os.fork():
         # this is the parent
-        rc = Follower().follow()
-        sys.exit(rc)
+        rc = Follower().follow(config['basedir'])
+        return rc
     # this is the child: give the logfile-watching parent a chance to start
     # watching it before we start the daemon
     time.sleep(0.2)
     launch(config)
 
 def launch(config):
-    sys.path.insert(0, os.path.abspath(os.getcwd()))
+    os.chdir(config['basedir'])
+    sys.path.insert(0, os.path.abspath(config['basedir']))
 
     # see if we can launch the application without actually having to
     # spawn twistd, since spawning processes correctly is a real hassle
@@ -103,4 +103,3 @@ def launch(config):
     # windows.
     from twisted.scripts import twistd
     twistd.run()
-

master/buildbot/test/test_extra_coverage.py

@@ -27,8 +27,8 @@
 modules.extend([base, sendchange, tryclient])
 from buildbot.process import mtrlogobserver, subunitlogobserver
 modules.extend([mtrlogobserver, subunitlogobserver])
-from buildbot.scripts import checkconfig, logwatcher, reconfig, runner, startup
-modules.extend([checkconfig, logwatcher, reconfig, runner, startup])
+from buildbot.scripts import checkconfig, logwatcher, reconfig, runner
+modules.extend([checkconfig, logwatcher, reconfig, runner])
 from buildbot.status import client, html, status_gerrit, status_push
 modules.extend([client, html, status_gerrit, status_push])
 from buildbot.status import tinderbox, words

master/buildbot/test/unit/test_scripts_restart.py

@@ -17,7 +17,7 @@
 
 import os
 from twisted.trial import unittest
-from buildbot.scripts import restart, stop, startup
+from buildbot.scripts import restart, stop, start
 from buildbot.test.util import dirs, misc
 
 def mkconfig(**kwargs):
@@ -48,17 +48,17 @@ def test_restart_stop_fails(self):
 
     def test_restart_stop_succeeds_start_fails(self):
         self.patch(stop, 'stop', lambda config, wait : 0)
-        self.patch(startup, 'start', lambda config : 1)
+        self.patch(start, 'start', lambda config : 1)
         self.assertEqual(restart.restart(mkconfig()), 1)
 
     def test_restart_succeeds(self):
         self.patch(stop, 'stop', lambda config, wait : 0)
-        self.patch(startup, 'start', lambda config : 0)
+        self.patch(start, 'start', lambda config : 0)
         self.assertEqual(restart.restart(mkconfig()), 0)
         self.assertStdout('now restarting')
 
     def test_restart_succeeds_quiet(self):
         self.patch(stop, 'stop', lambda config, wait : 0)
-        self.patch(startup, 'start', lambda config : 0)
+        self.patch(start, 'start', lambda config : 0)
         self.assertEqual(restart.restart(mkconfig(quiet=True)), 0)
         self.assertWasQuiet()

master/buildbot/test/unit/test_scripts_start.py

@@ -0,0 +1,63 @@
+# This file is part of Buildbot.  Buildbot is free software: you can
+# redistribute it and/or modify it under the terms of the GNU General Public
+# License as published by the Free Software Foundation, version 2.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program; if not, write to the Free Software Foundation, Inc., 51
+# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+# Copyright Buildbot Team Members
+
+from __future__ import with_statement
+
+import os
+from twisted.trial import unittest
+from buildbot.scripts import start
+from buildbot.test.util import dirs, misc
+
+def mkconfig(**kwargs):
+    config = dict(quiet=False, basedir=os.path.abspath('basedir'))
+    config.update(kwargs)
+    return config
+
+fake_master_tac = """\
+from twisted.application import service
+from twisted.python import log
+from twisted.internet import reactor
+application = service.Application('highscore')
+class App(service.Service):
+    def startService(self):
+        log.msg("BuildMaster is running") # heh heh heh
+        reactor.callLater(0, reactor.stop)
+app = App()
+app.setServiceParent(application)
+# isBuildmasterDir wants to see this -> Application('buildmaster')
+"""
+
+class TestStart(misc.StdoutAssertionsMixin, dirs.DirsMixin, unittest.TestCase):
+
+    def setUp(self):
+        self.setUpDirs('basedir')
+        with open(os.path.join('basedir', 'buildbot.tac'), 'wt') as f:
+            f.write(fake_master_tac)
+        self.setUpStdoutAssertions()
+
+    def tearDown(self):
+        self.tearDownDirs()
+
+    # tests
+
+    def test_start_not_basedir(self):
+        self.assertEqual(start.start(mkconfig(basedir='doesntexist')), 1)
+        self.assertStdout('not a buildmaster directory')
+
+    # the remainder of this script does obscene things:
+    #  - forks
+    #  - shells out to tail
+    #  - starts and stops the reactor
+    # so testing it will be *far* more pain than is worthwhile