Merge pull request #30 from frigg/return_code

Remove return code workaround and fix EOF newline behavior

Author: relekang

docker/helpers.py

@@ -34,8 +34,8 @@ def execute(cmd, stdin=''):
     )
 
     (stdout, stderr) = process.communicate(str.encode(stdin))
-    result.out = stdout.decode('utf-8').strip() if stdout else ''
-    result.err = stderr.decode('utf-8').strip() if stderr else ''
+    result.out = stdout.decode('utf-8') if stdout else ''
+    result.err = stderr.decode('utf-8') if stderr else ''
     result.return_code = process.returncode
     logger.debug('Finished running of: {0}'.format(result.__dict__))
     return result

docker/manager.py

@@ -1,5 +1,4 @@
 import logging
-import re
 import uuid
 from collections import OrderedDict
 from time import sleep
@@ -88,7 +87,7 @@ def run(self, command, working_directory='', stdin=''):
         ])
 
         result = execute(
-            'docker exec -i {container} bash -c \'{command} ;  echo "--return-$?--"\''.format(
+            'docker exec -i {container} bash -c \'{command}\''.format(
                 envs=env_string,
                 container=self.container_name,
                 command=command_string.format(
@@ -100,12 +99,6 @@ def run(self, command, working_directory='', stdin=''):
             stdin
         )
 
-        return_code_match = re.search(r'(?:\\n)?--return-(\d+)--$', result.out)
-        if return_code_match:
-            result.return_code = int(return_code_match.group(1))
-            result.out = result.out.replace(return_code_match.group(0), '')
-            if result.out.endswith('\n'):
-                result.out = result.out[:len(result.out) - 1]
         return result
 
     def read_file(self, path):
@@ -194,7 +187,7 @@ def list_files(self, path):
 
             raise errors.DockerWrapperBaseError(result.err)
 
-        return result.out.split('\n')
+        return result.out.strip().split('\n')
 
     def list_directories(self, path, include_trailing_slash=True):
         """
@@ -218,7 +211,7 @@ def list_directories(self, path, include_trailing_slash=True):
 
             raise errors.DockerWrapperBaseError(result.err)
 
-        for file_path in result.out.split(', '):
+        for file_path in result.out.strip().split(', '):
             if include_trailing_slash:
                 files.append(file_path)
             else:

tests/test_manager.py

@@ -69,8 +69,9 @@ def test_quotation_mark_handling(self, mock_run):
         docker = Docker()
         docker.run('echo "hi there"')
         docker.run("echo 'hi there'")
-        expected = ('docker exec -i {} bash -c \'cd ~/ &&  echo "hi there" ;  '
-                    'echo "--return-$?--"\''.format(docker.container_name), '')
+        expected = (
+            'docker exec -i {} bash -c \'cd ~/ &&  echo "hi there"\''.format(docker.container_name),
+            '')
 
         mock_run.assert_has_calls([mock.call(*expected), mock.call(*expected)])
 
@@ -79,9 +80,9 @@ def test_quotation_mark_handling(self, mock_run):
     def test_env_variables(self, mock_run):
         docker = Docker(env_variables={'CI': 1, 'FRIGG': 1})
         docker.run('ls')
-        mock_run.assert_called_once_with('docker exec -i {} bash -c \'cd ~/ && CI=1 FRIGG=1 ls ;'
-                                         '  echo "--return-$?--"\''.format(docker.container_name),
-                                         '')
+        mock_run.assert_called_once_with(
+            'docker exec -i {} bash -c \'cd ~/ && CI=1 FRIGG=1 ls\''.format(docker.container_name),
+            '')
 
     @mock.patch('docker.manager.execute')
     def test_single_port_mappping(self, mock_run):
@@ -206,9 +207,8 @@ def test_create_and_list_files_in_sub_directory(self):
 
     def test_read_file_with_content(self):
         file_name = 'readme.txt'
-        file_content = 'this is a test file {0}'.format(randint(5000, 5500))
-        self.docker.run('echo \"{0}\" > ~/{1}; cat readme.txt'.format(file_content, file_name))
-
+        file_content = 'this is a test file {0}\n'.format(randint(5000, 5500))
+        self.docker.write_file(file_name, file_content)
         self.assertEqual(self.docker.read_file(file_name), file_content)
 
     def test_read_file_that_dont_exist(self):
@@ -220,6 +220,17 @@ def test_read_file_that_dont_exist(self):
             path
         )
 
+    def test_read_file_eof_newline(self):
+        path = '/etc/hostname'
+        content = self.docker.read_file(path)
+        self.assertTrue(content.endswith('\n'))
+
+    def test_write_file_read_file(self):
+        path = 'testfile'
+        content = 'this is a nice file\n'
+        self.docker.write_file(path, content)
+        self.assertEqual(content, self.docker.read_file(path))
+
     def test_directory_exist(self):
         self.assertTrue(self.docker.directory_exist('~/'))
         self.assertFalse(self.docker.directory_exist('does-not-exist'))
@@ -233,15 +244,16 @@ def test_combine_output(self):
         self.docker.combine_outputs = True
         result = self.docker.run('ls does-not-exist')
         self.assertEqual(result.err, '')
-        self.assertEqual(result.out, 'ls: cannot access does-not-exist: No such file or directory')
+        self.assertEqual(result.out,
+                         'ls: cannot access does-not-exist: No such file or directory\n')
 
     def test_privilege(self):
         Docker(privilege=True).start()
 
     def test_write_file_append(self):
         path = 'readme.txt'
-        old_content = 'hi'
-        content = 'this is a readme'
+        old_content = 'hi\n'
+        content = 'this is a readme\n'
         self.docker.run('echo "{0}" > {1}'.format(old_content, path))
 
         self.docker.write_file(path, content, append=True)
@@ -251,7 +263,7 @@ def test_write_file_append(self):
     def test_write_file_no_append(self):
         path = 'readme.txt'
         old_content = 'hi'
-        content = 'this is a readme'
+        content = 'this is a readme\n'
         self.docker.run('echo "{0}" > {1}'.format(old_content, path))
 
         self.docker.write_file(path, content, append=False)
@@ -260,8 +272,16 @@ def test_write_file_no_append(self):
 
     def test_write_file_quotes(self):
         path = 'readme.txt'
-        content = 'this is a "readme"'
+        content = 'this is a "readme"\n'
 
         self.docker.write_file(path, content, append=False)
         written_content = self.docker.read_file(path)
         self.assertEqual(written_content, content)
+
+    def test_run_return_code(self):
+        code = 4
+        path = 'testfile'
+        content = 'exit {0}\n'.format(code)
+        self.docker.write_file(path, content)
+        result = self.docker.run('bash {0}'.format(path))
+        self.assertEqual(code, result.return_code)