Merge pull request #3 from malini/review

review related fixes

ci/dispatcher.py

@@ -52,11 +52,12 @@ class DispatcherHandler(SocketServer.BaseRequestHandler):
     and handle their requests and test results
     """
 
-    command_re = re.compile(r"""(\w*)([:]*(.*))""")
+    command_re = re.compile(r"(\w+)(:.+)*")
+    BUF_SIZE = 1024
 
     def handle(self):
         # self.request is the TCP socket connected to the client
-        self.data = self.request.recv(1024).strip()
+        self.data = self.request.recv(self.BUF_SIZE).strip()
         command_groups = self.command_re.match(self.data)
         if not command_groups:
             self.request.sendall("Invalid command")
@@ -67,7 +68,7 @@ def handle(self):
             self.request.sendall("OK")
         elif command == "dispatch":
             print "going to dispatch"
-            commit_hash = command_groups.group(3)
+            commit_hash = command_groups.group(2)[1:]
             if not self.server.runners:
                 self.request.sendall("No runners are registered")
             else:
@@ -84,16 +85,21 @@ def handle(self):
             self.request.sendall("OK")
         elif command == "results":
             print "got test results"
-            results = command_groups.group(2)
-            commit_hash = re.findall(r":(\w*):.*", results)[0]
+            results = command_groups.group(2)[1:]
+            results = results.split(":")
+            commit_hash = results[0]
+            length_msg = int(results[1])
+            # 3 is the number of ":" in the sent command
+            remaining_buffer = self.BUF_SIZE - (len(command) + len(commit_hash) + len(results[1]) + 3)
+            if length_msg > remaining_buffer:
+                self.data += self.request.recv(length_msg - remaining_buffer).strip()
             del self.server.dispatched_commits[commit_hash]
             if not os.path.exists("test_results"):
                 os.makedirs("test_results")
             with open("test_results/%s" % commit_hash, "w") as f:
-                data = self.data.split(":")[1:]
+                data = self.data.split(":")[3:]
                 data = "\n".join(data)
                 f.write(data)
-                f.close()
             self.request.sendall("OK")
         else:
             self.request.sendall("Invalid command")

ci/repo_observer.py

@@ -33,38 +33,38 @@ def poll():
             # call the bash script that will update the repo and check
             # for changes. If there's a change, it will drop a .commit_hash file
             # with the latest commit in the current working directory
-            subprocess.check_output(["./update_repo.sh %s" % args.repo],
-                                    shell=True)
-            if os.path.isfile(".commit_hash"):
-                # great, we have a change! let's execute the tests
-                # First, check the status of the dispatcher server to see
-                # if we can send the tests
-                try:
-                    response = helpers.communicate(dispatcher_host,
-                                                   int(dispatcher_port),
-                                                   "status")
-                except socket.error as e:
-                    raise Exception("Could not communicate with dispatcher server: %s" % e)
-                if response == "OK":
-                    # Dispatcher is present, let's send it a test
-                    commit = ""
-                    with open(".commit_hash", "r") as f:
-                        commit = f.readline()
-                    response = helpers.communicate(dispatcher_host,
-                                                   int(dispatcher_port),
-                                                   "dispatch:%s" % commit)
-                    if response != "OK":
-                        raise Exception("Could not dispatch the test: %s" %
-                        response)
-                    print "dispatched!"
-                else:
-                    # Something wrong happened to the dispatcher
-                    raise Exception("Could not dispatch the test: %s" %
-                    response)
-            time.sleep(5)
+            subprocess.check_output(["./update_repo.sh", args.repo])
         except subprocess.CalledProcessError as e:
             raise Exception("Could not update and check repository. Reason: %s" % e.output)
 
+        if os.path.isfile(".commit_hash"):
+            # great, we have a change! let's execute the tests
+            # First, check the status of the dispatcher server to see
+            # if we can send the tests
+            try:
+                response = helpers.communicate(dispatcher_host,
+                                               int(dispatcher_port),
+                                               "status")
+            except socket.error as e:
+                raise Exception("Could not communicate with dispatcher server: %s" % e)
+            if response == "OK":
+                # Dispatcher is present, let's send it a test
+                commit = ""
+                with open(".commit_hash", "r") as f:
+                    commit = f.readline()
+                response = helpers.communicate(dispatcher_host,
+                                               int(dispatcher_port),
+                                               "dispatch:%s" % commit)
+                if response != "OK":
+                    raise Exception("Could not dispatch the test: %s" %
+                    response)
+                print "dispatched!"
+            else:
+                # Something wrong happened to the dispatcher
+                raise Exception("Could not dispatch the test: %s" %
+                response)
+        time.sleep(5)
+
 
 if __name__ == "__main__":
     poll()

ci/run_or_fail.sh

@@ -1,10 +1,10 @@
 # helper method for providing error messages for a command
 run_or_fail() {
-  EXPLANATION=$1
+  local explanation=$1
   shift 1
-  $@
+  "$@"
   if [ $? != 0 ]; then
-    echo $EXPLANATION
+    echo $explanation
     exit 1
   fi
 }

ci/test_runner.py

@@ -9,6 +9,7 @@
 dispatcher.
 """
 import argparse
+import errno
 import os
 import re
 import socket
@@ -33,7 +34,7 @@ class TestHandler(SocketServer.BaseRequestHandler):
     The RequestHandler class for our server.
     """
 
-    command_re = re.compile(r"""(\w*)(?::(\w*))*""")
+    command_re = re.compile(r"(\w+)(:.+)*")
 
     def handle(self):
         # self.request is the TCP socket connected to the client
@@ -54,19 +55,21 @@ def handle(self):
             else:
                 self.request.sendall("OK")
                 print "running"
-                commit_hash = command_groups.group(2)
+                commit_hash = command_groups.group(2)[1:]
                 self.server.busy = True
                 self.run_tests(commit_hash,
                                self.server.repo_folder)
                 self.server.busy = False
+        else:
+            self.request.sendall("Invalid command")
 
     def run_tests(self, commit_hash, repo_folder):
         # update repo
-        output = subprocess.check_output(["./test_runner_script.sh %s %s" %
-                                        (repo_folder, commit_hash)], shell=True)
+        output = subprocess.check_output(["./test_runner_script.sh",
+                                        repo_folder, commit_hash])
         print output
         # run the tests
-        test_folder = os.path.sep.join([repo_folder, "tests"])
+        test_folder = os.path.join(repo_folder, "tests")
         suite = unittest.TestLoader().discover(test_folder)
         result_file = open("results", "w")
         unittest.TextTestRunner(result_file).run(suite)
@@ -76,17 +79,18 @@ def run_tests(self, commit_hash, repo_folder):
         output = result_file.read()
         helpers.communicate(self.server.dispatcher_server["host"],
                             int(self.server.dispatcher_server["port"]),
-                            "results:%s:%s" % (commit_hash, output))
+                            "results:%s:%s:%s" % (commit_hash, len(output), output))
 
 
 def serve():
+    range_start = 8900
     parser = argparse.ArgumentParser()
     parser.add_argument("--host",
                         help="runner's host, by default it uses localhost",
                         default="localhost",
                         action="store")
     parser.add_argument("--port",
-                        help="runner's port, by default it uses values >=8900",
+                        help="runner's port, by default it uses values >=%s" % range_start,
                         action="store")
     parser.add_argument("--dispatcher-server",
                         help="dispatcher host:port, by default it uses " \
@@ -101,7 +105,7 @@ def serve():
     runner_port = None
     tries = 0
     if not args.port:
-        runner_port = 8900
+        runner_port = range_start
         while tries < 100:
             try:
                 server = ThreadingTCPServer((runner_host, runner_port),
@@ -110,14 +114,14 @@ def serve():
                 print runner_port
                 break
             except socket.error as e:
-                if e.errno == 48:
+                if e.errno == errno.EADDRINUSE:
                     tries += 1
                     runner_port = runner_port + tries
                     continue
                 else:
                     raise e
         else:
-            raise Exception("Could not bind to ports in range 8900-9000")
+            raise Exception("Could not bind to ports in range %s-%s" % (range_start, range_start+tries))
     else:
         runner_port = int(args.port)
         server = ThreadingTCPServer((runner_host, runner_port), TestHandler)

ci/test_runner_script.sh

@@ -4,7 +4,7 @@ COMMIT=$2
 
 source run_or_fail.sh
 
-run_or_fail "Repository folder not found" pushd $REPO
+run_or_fail "Repository folder not found" pushd "$REPO" 1> /dev/null
 run_or_fail "Could not clean repository" git clean -d -f -x
 run_or_fail "Could not call git pull" git pull
-run_or_fail "Could not update to given commit hash" git reset --hard $COMMIT
+run_or_fail "Could not update to given commit hash" git reset --hard "$COMMIT"

ci/update_repo.sh

@@ -3,16 +3,14 @@
 source run_or_fail.sh
 
 # delete previous hash
-if [ -e ".commit_hash" ]; then
-  rm -f .commit_hash
-fi
+rm -f .commit_hash
 
 # go to repo and update it to given commit
-run_or_fail "Repository folder not found!" pushd $1
+run_or_fail "Repository folder not found!" pushd $1 1> /dev/null
 run_or_fail "Could not reset git" git reset --hard HEAD
 
 # get the most recent commit
-COMMIT=`git log -n1`
+COMMIT=$(run_or_fail "Could not call 'git log' on repository" git log -n1)
 if [ $? != 0 ]; then
   echo "Could not call 'git log' on repository"
   exit 1
@@ -24,7 +22,7 @@ HASH=`echo $COMMIT | awk '{ print $2 }'`
 run_or_fail "Could not pull from repository" git pull
 
 # get the most recent commit
-COMMIT=`git log -n1`
+COMMIT=$(run_or_fail "Could not call 'git log' on repository" git log -n1)
 if [ $? != 0 ]; then
   echo "Could not call 'git log' on repository"
   exit 1
@@ -34,6 +32,6 @@ NEWHASH=`echo $COMMIT | awk '{ print $2 }'`
 
 # if the hash changed, then write it to a file
 if [ $NEWHASH != $HASH ]; then
-  popd
+  popd 1> /dev/null
   echo $NEWHASH > .commit_hash
 fi