transfer steps: broke out slave check code

Broke out code to check slave version to it's own function.

master/buildbot/steps/transfer.py

@@ -189,6 +189,13 @@ class _TransferBuildStep(BuildStep):
     haltOnFailure = True
     flunkOnFailure = True
 
+    # Check that buildslave version used have implementation for
+    # a remote command. Raise exception if buildslave is to old.
+    def checkSlaveVersion(self, command):
+        if not self.slaveVersion(command):
+            message = "slave is too old, does not know about %s" % command
+            raise BuildSlaveTooOldError(message)
+
     def setDefaultWorkdir(self, workdir):
         if self.workdir is None:
             self.workdir = workdir
@@ -243,11 +250,7 @@ def __init__(self, slavesrc, masterdest,
         self.url = url
 
     def start(self):
-        version = self.slaveVersion("uploadFile")
-
-        if not version:
-            m = "slave is too old, does not know about uploadFile"
-            raise BuildSlaveTooOldError(m)
+        self.checkSlaveVersion("uploadFile")
 
         source = self.slavesrc
         masterdest = self.masterdest
@@ -313,11 +316,7 @@ def __init__(self, slavesrc, masterdest,
         self.url = url
 
     def start(self):
-        version = self.slaveVersion("uploadDirectory")
-
-        if not version:
-            m = "slave is too old, does not know about uploadDirectory"
-            raise BuildSlaveTooOldError(m)
+        self.checkSlaveVersion("uploadDirectory")
 
         source = self.slavesrc
         masterdest = self.masterdest
@@ -421,10 +420,7 @@ def __init__(self, mastersrc, slavedest,
         self.mode = mode
 
     def start(self):
-        version = self.slaveVersion("downloadFile")
-        if not version:
-            m = "slave is too old, does not know about downloadFile"
-            raise BuildSlaveTooOldError(m)
+        self.checkSlaveVersion("downloadFile")
 
         # we are currently in the buildmaster's basedir, so any non-absolute
         # paths will be interpreted relative to that
@@ -486,10 +482,8 @@ def __init__(self, s, slavedest,
         self.mode = mode
 
     def start(self):
-        version = self.slaveVersion("downloadFile")
-        if not version:
-            m = "slave is too old, does not know about downloadFile"
-            raise BuildSlaveTooOldError(m)
+        # we use 'downloadFile' remote command on the slave
+        self.checkSlaveVersion("downloadFile")
 
         # we are currently in the buildmaster's basedir, so any non-absolute
         # paths will be interpreted relative to that

master/buildbot/test/unit/test_steps_transfer.py

@@ -22,14 +22,48 @@
 
 from mock import Mock
 
+from buildbot.process import buildstep
 from buildbot.process.properties import Properties
 from buildbot.util import json
 from buildbot.steps import transfer
 from buildbot.status.results import SUCCESS
 from buildbot import config
+from buildbot import interfaces
 from buildbot.test.util import steps
 from buildbot.test.fake.remotecommand import Expect, ExpectRemoteRef
 
+
+# Test buildbot.steps.transfer._TransferBuildStep class.
+class TestTransferBuildStep(unittest.TestCase):
+
+    # Test calling checkSlaveVersion() when buildslave have support for
+    # requested remote command.
+    def testCheckSlaveVersionGood(self):
+        # patch BuildStep.slaveVersion() to return success
+        mockedSlaveVersion = Mock()
+        self.patch(buildstep.BuildStep, "slaveVersion", mockedSlaveVersion)
+
+        # check that no exceptions are raised
+        transfer._TransferBuildStep().checkSlaveVersion("foo")
+
+        # make sure slaveVersion() was called with correct arguments
+        mockedSlaveVersion.assert_called_once_with("foo")
+
+    # Test calling checkSlaveVersion() when buildslave is to old to support
+    # requested remote command.
+    def testCheckSlaveVersionTooOld(self):
+        # patch BuildStep.slaveVersion() to return error
+        self.patch(buildstep.BuildStep,
+                   "slaveVersion",
+                   Mock(return_value=None))
+
+        # make sure appropriate exception is raised
+        step = transfer._TransferBuildStep()
+        self.assertRaisesRegexp(interfaces.BuildSlaveTooOldError,
+                                "slave is too old, does not know about foo",
+                                step.checkSlaveVersion, "foo")
+
+
 class TestFileUpload(unittest.TestCase):
     def setUp(self):
         fd, self.destfile = tempfile.mkstemp()