From 119901c19b8c6a20e0113c84d9f5a3aac06310d6 Mon Sep 17 00:00:00 2001 From: Mazz Mosley Date: Wed, 22 Jul 2015 18:06:43 +0100 Subject: [PATCH 1/2] Improve test coverage for scale Also includes tiny amount of code cleanup, being explicit with imports. Signed-off-by: Mazz Mosley --- tests/integration/service_test.py | 124 ++++++++++++++++++++++++++++-- 1 file changed, 119 insertions(+), 5 deletions(-) diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index dbb97d8f3..97c06a9dc 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -4,10 +4,10 @@ import os from os import path from docker.errors import APIError -import mock +from mock import patch import tempfile import shutil -import six +from six import StringIO, text_type from compose import __version__ from compose.const import ( @@ -221,7 +221,7 @@ class ServiceTest(DockerClientTestCase): self.assertTrue(path.basename(actual_host_path) == path.basename(host_path), msg=("Last component differs: %s, %s" % (actual_host_path, host_path))) - @mock.patch.dict(os.environ) + @patch.dict(os.environ) def test_create_container_with_home_and_env_var_in_volume_path(self): os.environ['VOLUME_NAME'] = 'my-volume' os.environ['HOME'] = '/tmp/home-dir' @@ -469,7 +469,7 @@ class ServiceTest(DockerClientTestCase): with open(os.path.join(base_dir, b'foo\xE2bar'), 'w') as f: f.write("hello world\n") - self.create_service('web', build=six.text_type(base_dir)).build() + self.create_service('web', build=text_type(base_dir)).build() self.assertEqual(len(self.client.images(name='composetest_web')), 1) def test_start_container_stays_unpriviliged(self): @@ -549,6 +549,120 @@ class ServiceTest(DockerClientTestCase): service.scale(0) self.assertEqual(len(service.containers()), 0) + @patch('sys.stdout', new_callable=StringIO) + def test_scale_with_stopped_containers(self, mock_stdout): + """ + Given there are some stopped containers and scale is called with a + desired number that is the same as the number of stopped containers, + test that those containers are restarted and not removed/recreated. + """ + service = self.create_service('web') + next_number = service._next_container_number() + valid_numbers = [next_number, next_number + 1] + service.create_container(number=next_number, quiet=True) + service.create_container(number=next_number + 1, quiet=True) + + for container in service.containers(): + self.assertFalse(container.is_running) + + service.scale(2) + + self.assertEqual(len(service.containers()), 2) + for container in service.containers(): + self.assertTrue(container.is_running) + self.assertTrue(container.number in valid_numbers) + + captured_output = mock_stdout.getvalue() + self.assertNotIn('Creating', captured_output) + self.assertIn('Starting', captured_output) + + @patch('sys.stdout', new_callable=StringIO) + def test_scale_with_stopped_containers_and_needing_creation(self, mock_stdout): + """ + Given there are some stopped containers and scale is called with a + desired number that is greater than the number of stopped containers, + test that those containers are restarted and required number are created. + """ + service = self.create_service('web') + next_number = service._next_container_number() + service.create_container(number=next_number, quiet=True) + + for container in service.containers(): + self.assertFalse(container.is_running) + + service.scale(2) + + self.assertEqual(len(service.containers()), 2) + for container in service.containers(): + self.assertTrue(container.is_running) + + captured_output = mock_stdout.getvalue() + self.assertIn('Creating', captured_output) + self.assertIn('Starting', captured_output) + + @patch('sys.stdout', new_callable=StringIO) + def test_scale_with_api_returns_errors(self, mock_stdout): + """ + Test that when scaling if the API returns an error, that error is handled + and the remaining threads continue. + """ + service = self.create_service('web') + next_number = service._next_container_number() + service.create_container(number=next_number, quiet=True) + + with patch( + 'compose.container.Container.create', + side_effect=APIError(message="testing", response={}, explanation="Boom")): + + service.scale(3) + + self.assertEqual(len(service.containers()), 1) + self.assertTrue(service.containers()[0].is_running) + self.assertIn("ERROR: for 2 Boom", mock_stdout.getvalue()) + + @patch('compose.service.log') + def test_scale_with_desired_number_already_achieved(self, mock_log): + """ + Test that calling scale with a desired number that is equal to the + number of containers already running results in no change. + """ + service = self.create_service('web') + next_number = service._next_container_number() + container = service.create_container(number=next_number, quiet=True) + container.start() + + self.assertTrue(container.is_running) + self.assertEqual(len(service.containers()), 1) + + service.scale(1) + + self.assertEqual(len(service.containers()), 1) + container.inspect() + self.assertTrue(container.is_running) + + captured_output = mock_log.info.call_args[0] + self.assertIn('Desired container number already achieved', captured_output) + + @patch('compose.service.log') + def test_scale_with_custom_container_name_outputs_warning(self, mock_log): + """ + Test that calling scale on a service that has a custom container name + results in warning output. + """ + service = self.create_service('web', container_name='custom-container') + + self.assertEqual(service.custom_container_name(), 'custom-container') + + service.scale(3) + + captured_output = mock_log.warn.call_args[0][0] + + self.assertEqual(len(service.containers()), 1) + self.assertIn( + "Remove the custom name to scale the service.", + captured_output + ) + def test_scale_sets_ports(self): service = self.create_service('web', ports=['8000']) service.scale(2) @@ -650,7 +764,7 @@ class ServiceTest(DockerClientTestCase): for k, v in {'ONE': '1', 'TWO': '2', 'THREE': '3', 'FOO': 'baz', 'DOO': 'dah'}.items(): self.assertEqual(env[k], v) - @mock.patch.dict(os.environ) + @patch.dict(os.environ) def test_resolve_env(self): os.environ['FILE_DEF'] = 'E1' os.environ['FILE_DEF_EMPTY'] = 'E2' From 2c8aade13e886e450e7226340c115a4641c07586 Mon Sep 17 00:00:00 2001 From: Mazz Mosley Date: Wed, 22 Jul 2015 18:07:56 +0100 Subject: [PATCH 2/2] Space for errors It was harder to see when there are errors if they came straight after the other output. Putting a newline in there gives it a bit of visual room. Signed-off-by: Mazz Mosley --- compose/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/compose/utils.py b/compose/utils.py index ff3096fd2..4c7f94c57 100644 --- a/compose/utils.py +++ b/compose/utils.py @@ -57,6 +57,7 @@ def parallel_execute(objects, obj_callable, msg_index, msg): pass if errors: + stream.write("\n") for error in errors: stream.write("ERROR: for {} {} \n".format(error, errors[error]))