Merge lp:~timrchavez/offspring/fix-lp-1133432 into lp:offspring

[Timothy R. Chavez]

Add logging for failure conditions in getBuildName, so problems can be be properly investigated and resolved more quickly.

[lp:~timrchavez/offspring/fix-lp-1133432]

166. By Timothy R. Chavez on 2013-06-04

Disambiguate error messages and add a test explicitly for the case we're now logging.

167. By Timothy R. Chavez on 2013-06-04

Remove crufty imports and fix log message.

[Kevin McDermott (bigkevmcd)]

Looks good to me, +1

review: Approve

=== modified file 'lib/offspring/slave/slave.py'
--- lib/offspring/slave/slave.py	2012-04-11 06:59:00 +0000
+++ lib/offspring/slave/slave.py	2013-06-04 17:04:35 +0000
@@ -2,8 +2,6 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 import logging
 import platform
-import re
-import tempfile
 import time
 import xmlrpclib
 
@@ -283,6 +281,7 @@
                     if l.startswith("START"):
                         self.buildName = l.split(" ")[3].strip()
                         return self.buildName
+                logging.error(self.stderrdata)
             else:
                 try:
                     lfile = open(self.master.config.get("slave",
@@ -295,6 +294,9 @@
                 if projectName == self.getProjectName():
                     self.buildName = buildName.strip("\n")
                     return self.buildName
+                logging.error(
+                    "Error when trying to determine build name due to project "
+                    "name mismatch")
         return LexbuilderSlaveServer.REQUEST_ERROR
 
 

=== modified file 'lib/offspring/slave/tests/test_slave.py'
--- lib/offspring/slave/tests/test_slave.py	2012-04-11 06:59:00 +0000
+++ lib/offspring/slave/tests/test_slave.py	2013-06-04 17:04:35 +0000
@@ -2,10 +2,7 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 import logging
-import os
 import platform
-import re
-import subprocess
 import xmlrpclib
 from datetime import datetime
 
@@ -18,6 +15,7 @@
 from offspring.slave import (
     LexbuilderSlave, ProjectBuildProcess, LexbuilderSlaveServer,
     ShutdownThread)
+from offspring.slave.taskmonitor import MonitoredProcess
 from offspring.config import setup_logging
 
 
@@ -274,7 +272,7 @@
         log_file = self.capture_logging(log_level=logging.DEBUG)
         self.config.set("slave", "request_timeout", "30.0")
 
-        slave_server = LexbuilderSlaveServer(
+        LexbuilderSlaveServer(
             self.fake_slave, ("localhost", 8760), self.config)
         self.assertEqual("Request timeout set to 30.0 seconds\n",
                          log_file.getvalue())
@@ -288,7 +286,7 @@
         self.config.set("slave", "request_timeout", "broken value")
 
 
-        slave_server = LexbuilderSlaveServer(
+        LexbuilderSlaveServer(
             self.fake_slave, ("localhost", 8760), self.config)
         self.assertEqual("Request timeout set to 60.0 seconds\n",
                          log_file.getvalue())
@@ -706,7 +704,7 @@
     def test_get_build_name_with_unknown_build_active_process(self):
         """
         If the build name is unknown, and the process is currently active,
-        then we ProjectBuildProcess.getBuildName should examine the the build
+        then we ProjectBuildProcess.getBuildName should examine the build
         statefile from the configuration and parse the project name and build
         name from there.
         """
@@ -737,16 +735,51 @@
     def test_get_build_name_with_unknown_build_active_process_error(self):
         """
         If the build name is unknown, and the process is currently active,
-        then we ProjectBuildProcess.getBuildName should examine the the build
+        then we ProjectBuildProcess.getBuildName should examine the build
         statefile from the configuration, any error reading this should result
         in an error return.
         """
+        log_file = self.capture_logging()
         self.build_process.state = self.build_process.STATE_ACTIVE
         self.build_process.projectName = "newproject"
         build_statefile_name = self.makeFile()
         self.config.set("slave", "build_statefile", build_statefile_name)
         self.assertEqual(LexbuilderSlaveServer.REQUEST_ERROR,
                          self.build_process.getBuildName())
+        self.assertTrue(
+            "Error when trying to determine build name" in log_file.getvalue())
+
+    def test_get_build_name_with_unknown_build_active_process_error_mismatch(self):
+        """
+        If the build name is unknown, and the process is currently active,
+        then we ProjectBuildProcess.getBuildName should examine the build
+        statefile from the configuration and parse the project name and build
+        name from there. An error results if the statefile projectname does not
+        match the process's project name.
+        """
+        log_file = self.capture_logging()
+        self.build_process.state = self.build_process.STATE_ACTIVE
+        self.build_process.projectName = "newproject"
+        build_statefile_name = self.makeFile("oldproject oldbuildname")
+        self.config.set("slave", "build_statefile", build_statefile_name)
+        self.assertEqual(LexbuilderSlaveServer.REQUEST_ERROR,
+                         self.build_process.getBuildName())
+        self.assertTrue(
+            "Error when trying to determine build name due to project "
+            "name mismatch" in log_file.getvalue())
+
+    def test_get_build_name_error_before_build_starts(self):
+        """
+        Test that errors which occur in shell scripts before a build starts
+        are logged.
+        """
+        log_file = self.capture_logging()
+        self.build_process.state = MonitoredProcess.STATE_FAILED
+        self.build_process.stdoutdata = ""
+        self.build_process.stderrdata = "This is a shell script error."
+        self.build_process.getBuildName()
+        self.assertTrue(
+            "This is a shell script error." in log_file.getvalue())
 
 
 class TestShutdownThread(MockerTestCase):