launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:codeimport-bytesio into launchpad:master.

Commit message:
Port lp.codehosting.codeimport to io.BytesIO

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/388007
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:codeimport-bytesio into launchpad:master.

diff --git a/lib/lp/codehosting/codeimport/tests/servers.py b/lib/lp/codehosting/codeimport/tests/servers.py
index 3a872cb..a868e88 100644
--- a/lib/lp/codehosting/codeimport/tests/servers.py
+++ b/lib/lp/codehosting/codeimport/tests/servers.py
@@ -12,8 +12,8 @@ __all__ = [
 
 __metaclass__ = type
 
-from cStringIO import StringIO
 import errno
+import io
 import os
 import re
 import shutil
@@ -184,8 +184,8 @@ class SubversionServer(Server):
         for filename, content in tree_contents:
             f = root.add_file(filename)
             try:
-                subvertpy.delta.send_stream(StringIO(content),
-                    f.apply_textdelta())
+                subvertpy.delta.send_stream(
+                    io.BytesIO(content), f.apply_textdelta())
             finally:
                 f.close()
         root.close()
diff --git a/lib/lp/codehosting/codeimport/tests/test_worker.py b/lib/lp/codehosting/codeimport/tests/test_worker.py
index 30a3e11..d0a30cf 100644
--- a/lib/lp/codehosting/codeimport/tests/test_worker.py
+++ b/lib/lp/codehosting/codeimport/tests/test_worker.py
@@ -848,7 +848,7 @@ class TestActualImportMixin:
         # Running the worker on a branch that hasn't been imported yet imports
         # the branch.
         worker = self.makeImportWorker(self.makeSourceDetails(
-            'trunk', [('README', 'Original contents')]),
+            'trunk', [('README', b'Original contents')]),
             opener_policy=AcceptAnythingPolicy())
         worker.run()
         branch = self.getStoredBazaarBranch(worker)
@@ -857,7 +857,7 @@ class TestActualImportMixin:
     def test_sync(self):
         # Do an import.
         worker = self.makeImportWorker(self.makeSourceDetails(
-            'trunk', [('README', 'Original contents')]),
+            'trunk', [('README', b'Original contents')]),
             opener_policy=AcceptAnythingPolicy())
         worker.run()
         branch = self.getStoredBazaarBranch(worker)
@@ -878,7 +878,7 @@ class TestActualImportMixin:
         # Like test_import, but using the code-import-worker.py script
         # to perform the import.
         arguments = self.makeWorkerArguments(
-            'trunk', [('README', 'Original contents')])
+            'trunk', [('README', b'Original contents')])
         source_details = CodeImportSourceDetails.fromArguments(arguments)
 
         clean_up_default_stores_for_import(source_details)
@@ -916,7 +916,7 @@ class TestActualImportMixin:
         # import that does not import revisions, the worker exits with a code
         # of CodeImportWorkerExitCode.SUCCESS_NOCHANGE.
         arguments = self.makeWorkerArguments(
-            'trunk', [('README', 'Original contents')])
+            'trunk', [('README', b'Original contents')])
         source_details = CodeImportSourceDetails.fromArguments(arguments)
 
         clean_up_default_stores_for_import(source_details)
@@ -1079,7 +1079,7 @@ class PullingImportWorkerTests:
     def test_unsupported_feature(self):
         # If there is no branch in the target URL, exit with FAILURE_INVALID
         worker = self.makeImportWorker(self.makeSourceDetails(
-            'trunk', [('bzr\\doesnt\\support\\this', 'Original contents')]),
+            'trunk', [('bzr\\doesnt\\support\\this', b'Original contents')]),
             opener_policy=AcceptAnythingPolicy())
         self.assertEqual(
             CodeImportWorkerExitCode.FAILURE_UNSUPPORTED_FEATURE,
@@ -1089,7 +1089,7 @@ class PullingImportWorkerTests:
         # Only config.codeimport.revisions_import_limit will be imported
         # in a given run.
         worker = self.makeImportWorker(self.makeSourceDetails(
-            'trunk', [('README', 'Original contents')]),
+            'trunk', [('README', b'Original contents')]),
             opener_policy=AcceptAnythingPolicy())
         self.makeForeignCommit(worker.source_details)
         self.assertTrue(self.foreign_commit_count > 1)
@@ -1107,7 +1107,7 @@ class PullingImportWorkerTests:
     def test_stacked(self):
         stacked_on = self.make_branch('stacked-on')
         source_details = self.makeSourceDetails(
-            'trunk', [('README', 'Original contents')],
+            'trunk', [('README', b'Original contents')],
             stacked_on_url=stacked_on.base)
         stacked_on.fetch(Branch.open(source_details.url))
         base_rev_count = self.foreign_commit_count
@@ -1196,7 +1196,7 @@ class TestGitImport(WorkerTest, TestActualImportMixin,
     def test_non_master(self):
         # non-master branches can be specified in the import URL.
         source_details = self.makeSourceDetails(
-            'trunk', [('README', 'Original contents')])
+            'trunk', [('README', b'Original contents')])
         self.makeForeignCommit(source_details, ref="refs/heads/other",
             message="Message for other")
         self.makeForeignCommit(source_details, ref="refs/heads/master",
@@ -1238,7 +1238,7 @@ class TestBzrSvnImport(WorkerTest, SubversionImportHelpers,
         # cache in the worker's ImportDataStore.
         from bzrlib.plugins.svn.cache import get_cache
         worker = self.makeImportWorker(self.makeSourceDetails(
-            'trunk', [('README', 'Original contents')]),
+            'trunk', [('README', b'Original contents')]),
             opener_policy=AcceptAnythingPolicy())
         uuid = subvertpy.ra.RemoteAccess(worker.source_details.url).get_uuid()
         cache_dir = get_cache(uuid).create_cache_dir()
@@ -1258,7 +1258,7 @@ class TestBzrSvnImport(WorkerTest, SubversionImportHelpers,
         # into the appropriate cache directory.
         from bzrlib.plugins.svn.cache import get_cache
         worker = self.makeImportWorker(self.makeSourceDetails(
-            'trunk', [('README', 'Original contents')]),
+            'trunk', [('README', b'Original contents')]),
             opener_policy=AcceptAnythingPolicy())
         # Store a tarred-up cache in the store.
         content = self.factory.getUniqueString()
diff --git a/lib/lp/codehosting/codeimport/tests/test_workermonitor.py b/lib/lp/codehosting/codeimport/tests/test_workermonitor.py
index 634a741..06635e3 100644
--- a/lib/lp/codehosting/codeimport/tests/test_workermonitor.py
+++ b/lib/lp/codehosting/codeimport/tests/test_workermonitor.py
@@ -8,9 +8,9 @@ __all__ = [
     'nuke_codeimport_sample_data',
     ]
 
+import io
 import os
 import shutil
-import StringIO
 import subprocess
 import tempfile
 
@@ -106,7 +106,7 @@ class TestWorkerMonitorProtocol(ProcessTestsMixin, TestCase):
 
     def setUp(self):
         self.worker_monitor = self.StubWorkerMonitor()
-        self.log_file = StringIO.StringIO()
+        self.log_file = io.BytesIO()
         super(TestWorkerMonitorProtocol, self).setUp()
 
     def makeProtocol(self):
@@ -152,7 +152,7 @@ class TestWorkerMonitorProtocol(ProcessTestsMixin, TestCase):
 
     def test_outReceivedWritesToLogFile(self):
         # outReceived writes the data it is passed into the log file.
-        output = ['some data\n', 'some more data\n']
+        output = [b'some data\n', b'some more data\n']
         self.protocol.outReceived(output[0])
         self.assertEqual(self.log_file.getvalue(), output[0])
         self.protocol.outReceived(output[1])
@@ -362,15 +362,15 @@ class TestWorkerMonitorUnit(TestCase):
         # finishJob method uploads the log file to the librarian and calls the
         # finishJobID XML-RPC method with the url of that file.
         self.layer.force_dirty_database()
-        log_text = self.factory.getUniqueString()
+        log_bytes = self.factory.getUniqueBytes()
         worker_monitor = self.makeWorkerMonitorWithJob()
-        worker_monitor._log_file.write(log_text)
+        worker_monitor._log_file.write(log_bytes)
 
         def check_file_uploaded(result):
             transaction.abort()
             url = worker_monitor.codeimport_endpoint.calls[0][3]
-            text = urlopen(url).read()
-            self.assertEqual(log_text, text)
+            got_log_bytes = urlopen(url).read()
+            self.assertEqual(log_bytes, got_log_bytes)
 
         return worker_monitor.finishJob(
             CodeImportResultStatus.SUCCESS).addCallback(
@@ -387,7 +387,7 @@ class TestWorkerMonitorUnit(TestCase):
         # Write some text so that we try to upload the log.
         job_id = self.factory.getUniqueInteger()
         worker_monitor = self.makeWorkerMonitorWithJob(job_id)
-        worker_monitor._log_file.write('some text')
+        worker_monitor._log_file.write(b'some text')
 
         # Make _createLibrarianFileAlias fail in a distinctive way.
         worker_monitor._createLibrarianFileAlias = FakeMethod(failure=Fail())
@@ -543,9 +543,9 @@ class TestWorkerMonitorUnit(TestCase):
 
         def check_log_file(ignored):
             worker_monitor._log_file.seek(0)
-            log_text = worker_monitor._log_file.read()
-            self.assertIn('Traceback (most recent call last)', log_text)
-            self.assertIn('RuntimeError', log_text)
+            log_bytes = worker_monitor._log_file.read()
+            self.assertIn(b'Traceback (most recent call last)', log_bytes)
+            self.assertIn(b'RuntimeError', log_bytes)
         return ret.addCallback(check_log_file)
 
     def test_callFinishJobRespects_call_finish_job(self):
@@ -652,17 +652,17 @@ class TestWorkerMonitorRunNoProcess(TestCase):
             publisher_adapter=oops_twisted.defer_publisher,
             publisher_helpers=oops_twisted.publishers)
         self.addCleanup(errorlog.globalErrorUtility.configure)
-        failure_msg = "test_callFinishJob_logs_failure expected failure"
+        failure_msg = b"test_callFinishJob_logs_failure expected failure"
         worker_monitor = self.WorkerMonitor(
             defer.fail(RuntimeError(failure_msg)))
         d = worker_monitor.run()
 
         def check_log_file(ignored):
             worker_monitor._log_file.seek(0)
-            log_text = worker_monitor._log_file.read()
+            log_bytes = worker_monitor._log_file.read()
             self.assertIn(
-                "Failure: exceptions.RuntimeError: " + failure_msg,
-                log_text)
+                b"Failure: exceptions.RuntimeError: " + failure_msg,
+                log_bytes)
 
         d.addCallback(check_log_file)
         return d
@@ -744,7 +744,7 @@ class TestWorkerMonitorIntegration(TestCaseInTempDir, TestCase):
         self.subversion_server.start_server()
         self.addCleanup(self.subversion_server.stop_server)
         url = self.subversion_server.makeBranch(
-            'trunk', [('README', 'contents')])
+            'trunk', [('README', b'contents')])
         self.foreign_commit_count = 2
 
         return self.factory.makeCodeImport(svn_branch_url=url)
@@ -756,7 +756,7 @@ class TestWorkerMonitorIntegration(TestCaseInTempDir, TestCase):
         self.subversion_server.start_server()
         self.addCleanup(self.subversion_server.stop_server)
         url = self.subversion_server.makeBranch(
-            'trunk', [('README', 'contents')])
+            'trunk', [('README', b'contents')])
         self.foreign_commit_count = 2
 
         return self.factory.makeCodeImport(
diff --git a/lib/lp/codehosting/codeimport/workermonitor.py b/lib/lp/codehosting/codeimport/workermonitor.py
index 2a2035a..80cbe4b 100644
--- a/lib/lp/codehosting/codeimport/workermonitor.py
+++ b/lib/lp/codehosting/codeimport/workermonitor.py
@@ -305,8 +305,8 @@ class CodeImportWorkerMonitor:
             return reason
         status = self._reasonToStatus(reason)
         if status == CodeImportResultStatus.FAILURE:
-            self._log_file.write("Import failed:\n")
-            reason.printTraceback(self._log_file)
+            self._log_file.write(b"Import failed:\n")
+            self._log_file.write(reason.getTraceback().encode("UTF-8"))
             self._logger.info(
                 "Import failed: %s: %s" % (reason.type, reason.value))
         else: