Merge pull request #1672 from jaredgrubb/jgrubb-string-download-fail-…

…nine

[nine] StringDownload: fix an exception that happens if the transfer fails

master/buildbot/steps/transfer.py

@@ -61,7 +61,7 @@ def runTransferCommand(self, cmd, writer=None):
 
         @d.addCallback
         def checkResult(_):
-            if cmd.didFail():
+            if writer and cmd.didFail():
                 writer.cancel()
             return FAILURE if cmd.didFail() else SUCCESS
 

master/buildbot/test/unit/test_steps_transfer.py

@@ -51,6 +51,19 @@ def behavior(command):
     return behavior
 
 
+def downloadString(memoizer, timestamp=None):
+    def behavior(command):
+        reader = command.args['reader']
+        read = reader.remote_read(1000)
+        # save what we read so we can check it
+        memoizer(read)
+        reader.remote_close()
+        if timestamp:
+            reader.remote_utime(timestamp)
+        return read
+    return behavior
+
+
 def uploadTarFile(filename, **members):
     def behavior(command):
         f = StringIO()
@@ -483,7 +496,13 @@ def checkCalls(res):
         return d
 
 
-class TestStringDownload(unittest.TestCase):
+class TestStringDownload(steps.BuildStepMixin, unittest.TestCase):
+
+    def setUp(self):
+        return self.setUpBuildStep()
+
+    def tearDown(self):
+        return self.tearDownBuildStep()
 
     # check that ConfigErrors is raised on invalid 'mode' argument
 
@@ -496,58 +515,91 @@ def testModeConfError(self):
             "string", "file", mode="not-a-number")
 
     def testBasic(self):
-        s = transfer.StringDownload("Hello World", "hello.txt")
-        s.build = Mock()
-        s.build.getProperties.return_value = Properties()
-        s.build.getSlaveCommandVersion.return_value = 1
+        self.setupStep(transfer.StringDownload("Hello World", "hello.txt"))
 
-        s.step_status = Mock()
-        s.buildslave = Mock()
-        s.remote = Mock()
+        self.step.buildslave = Mock()
+        self.step.remote = Mock()
 
-        s.start()
+        # A place to store what gets read
+        read = []
 
-        for c in s.remote.method_calls:
-            name, command, args = c
-            commandName = command[3]
-            kwargs = command[-1]
-            if commandName == 'downloadFile':
-                self.assertEquals(kwargs['slavedest'], 'hello.txt')
-                reader = kwargs['reader']
-                data = reader.remote_read(100)
-                self.assertEquals(data, "Hello World")
-                break
-        else:
-            self.assert_(False, "No downloadFile command found")
+        self.expectCommands(
+            Expect('downloadFile', dict(
+                slavedest="hello.txt", workdir='wkdir',
+                blocksize=16384, maxsize=None, mode=None,
+                reader=ExpectRemoteRef(remotetransfer.StringFileReader)))
+            + Expect.behavior(downloadString(read.append))
+            + 0)
 
+        self.expectOutcome(result=SUCCESS, state_string="downloading to hello.txt")
+        d = self.runStep()
+
+        @d.addCallback
+        def checkCalls(res):
+            self.assertEquals(''.join(read), "Hello World")
+        return d
+
+    def testFailure(self):
+        self.setupStep(transfer.StringDownload("Hello World", "hello.txt"))
+
+        self.expectCommands(
+            Expect('downloadFile', dict(
+                slavedest="hello.txt", workdir='wkdir',
+                blocksize=16384, maxsize=None, mode=None,
+                reader=ExpectRemoteRef(remotetransfer.StringFileReader)))
+            + 1)
 
-class TestJSONStringDownload(unittest.TestCase):
+        self.expectOutcome(result=FAILURE, state_string="downloading to hello.txt (failure)")
+        return self.runStep()
+
+
+class TestJSONStringDownload(steps.BuildStepMixin, unittest.TestCase):
+    def setUp(self):
+        return self.setUpBuildStep()
+
+    def tearDown(self):
+        return self.tearDownBuildStep()
 
     def testBasic(self):
         msg = dict(message="Hello World")
-        s = transfer.JSONStringDownload(msg, "hello.json")
-        s.build = Mock()
-        s.build.getProperties.return_value = Properties()
-        s.build.getSlaveCommandVersion.return_value = 1
+        self.setupStep(transfer.JSONStringDownload(msg, "hello.json"))
 
-        s.step_status = Mock()
-        s.buildslave = Mock()
-        s.remote = Mock()
+        self.step.buildslave = Mock()
+        self.step.remote = Mock()
 
-        s.start()
+        # A place to store what gets read
+        read = []
 
-        for c in s.remote.method_calls:
-            name, command, args = c
-            commandName = command[3]
-            kwargs = command[-1]
-            if commandName == 'downloadFile':
-                self.assertEquals(kwargs['slavedest'], 'hello.json')
-                reader = kwargs['reader']
-                data = reader.remote_read(100)
-                self.assertEquals(data, json.dumps(msg))
-                break
-        else:
-            self.assert_(False, "No downloadFile command found")
+        self.expectCommands(
+            Expect('downloadFile', dict(
+                slavedest="hello.json", workdir='wkdir',
+                blocksize=16384, maxsize=None, mode=None,
+                reader=ExpectRemoteRef(remotetransfer.StringFileReader))
+            )
+            + Expect.behavior(downloadString(read.append))
+            + 0)
+
+        self.expectOutcome(result=SUCCESS, state_string="downloading to hello.json")
+        d = self.runStep()
+
+        @d.addCallback
+        def checkCalls(res):
+            self.assertEquals(''.join(read), '{"message": "Hello World"}')
+        return d
+
+    def testFailure(self):
+        msg = dict(message="Hello World")
+        self.setupStep(transfer.JSONStringDownload(msg, "hello.json"))
+
+        self.expectCommands(
+            Expect('downloadFile', dict(
+                slavedest="hello.json", workdir='wkdir',
+                blocksize=16384, maxsize=None, mode=None,
+                reader=ExpectRemoteRef(remotetransfer.StringFileReader)))
+            + 1)
+
+        self.expectOutcome(result=FAILURE, state_string="downloading to hello.json (failure)")
+        return self.runStep()
 
 
 class TestJSONPropertiesDownload(unittest.TestCase):
@@ -563,7 +615,6 @@ def testBasic(self):
         ss.asDict.return_value = dict(revision="12345")
         s.build.getSourceStamp.return_value = ss
 
-        s.step_status = Mock()
         s.buildslave = Mock()
         s.remote = Mock()