From 0e3ddbfcba0264ae998d2a0132d2a6d5ef241ce1 Mon Sep 17 00:00:00 2001 From: Mikhail Dubov Date: Tue, 8 Oct 2013 11:43:09 +0400 Subject: [PATCH] Fixing incorrect mocks usage in unit tests This patch fixes some incorrect mocks usage in the unit tests for Rally. Those mocks were initialized in a wrong way (via the start() method in the test case setUp()) which presented an opportunity for a race condition. Here, this get replaced with "with ... as ..." blocks. Change-Id: Ifad16ee6454abcb42e05bb187842a6682d7924db Closes-Bug: #1236725 --- tests/benchmark/test_test_engine.py | 27 +++++++++---------- tests/benchmark/test_utils.py | 41 +++++++++++++++-------------- 2 files changed, 33 insertions(+), 35 deletions(-) diff --git a/tests/benchmark/test_test_engine.py b/tests/benchmark/test_test_engine.py index 275f5872f2..747ece3a3e 100644 --- a/tests/benchmark/test_test_engine.py +++ b/tests/benchmark/test_test_engine.py @@ -64,13 +64,6 @@ class TestEngineTestCase(test.NoDBTestCase): self.run_success = { 'proc': {'msg': ['msg'], 'status': 0, 'proc_name': 'proc'} } - self.run_mock = mock.patch('rally.benchmark.utils.Verifier.run', - mock.Mock(return_value=self.run_success)) - self.run_mock.start() - - def tearDown(self): - self.run_mock.stop() - super(TestEngineTestCase, self).tearDown() def test_verify_test_config(self): try: @@ -99,11 +92,13 @@ class TestEngineTestCase(test.NoDBTestCase): def test_verify(self): test_engine = engine.TestEngine(self.valid_test_config, mock.MagicMock()) - with test_engine.bind(self.valid_cloud_config): - try: - test_engine.verify() - except Exception as e: - self.fail("Exception in TestEngine.verify: %s" % str(e)) + with mock.patch('rally.benchmark.utils.Verifier.run') as mock_run: + mock_run.return_value = self.run_success + with test_engine.bind(self.valid_cloud_config): + try: + test_engine.verify() + except Exception as e: + self.fail("Exception in TestEngine.verify: %s" % str(e)) def test_benchmark(self): test_engine = engine.TestEngine(self.valid_test_config, @@ -114,9 +109,11 @@ class TestEngineTestCase(test.NoDBTestCase): def test_task_status_basic_chain(self): fake_task = mock.MagicMock() test_engine = engine.TestEngine(self.valid_test_config, fake_task) - with test_engine.bind(self.valid_cloud_config): - test_engine.verify() - test_engine.benchmark() + with mock.patch('rally.benchmark.utils.Verifier.run') as mock_run: + mock_run.return_value = self.run_success + with test_engine.bind(self.valid_cloud_config): + test_engine.verify() + test_engine.benchmark() s = consts.TaskStatus expected = [ diff --git a/tests/benchmark/test_utils.py b/tests/benchmark/test_utils.py index ca226df7fe..50c6d0e5a1 100644 --- a/tests/benchmark/test_utils.py +++ b/tests/benchmark/test_utils.py @@ -169,27 +169,25 @@ def test_dummy_timeout(): class VerifierTestCase(test.NoDBTestCase): def setUp(self): super(VerifierTestCase, self).setUp() - self.fc = mock.patch('fuel_health.cleanup.cleanup') - self.fc.start() self.cloud_config_manager = config.CloudConfigManager() self.cloud_config_path = os.path.abspath('dummy_test.conf') with open(self.cloud_config_path, 'w') as f: self.cloud_config_manager.write(f) def tearDown(self): - self.fc.stop() if os.path.exists(self.cloud_config_path): os.remove(self.cloud_config_path) super(VerifierTestCase, self).tearDown() def test_running_test(self): tester = utils.Verifier(mock.MagicMock(), self.cloud_config_path) - test = ['./tests/benchmark/test_utils.py', '-k', 'test_dummy_1'] - for (times, concurrent) in [(1, 1), (3, 2), (2, 3)]: - results = tester.run(test, times=times, concurrent=concurrent) - self.assertEqual(len(results), times) - for result in results.itervalues(): - self.assertEqual(result['status'], 0) + with mock.patch('rally.benchmark.utils.fuel_cleanup.cleanup'): + test = ['./tests/benchmark/test_utils.py', '-k', 'test_dummy_1'] + for (times, concurrent) in [(1, 1), (3, 2), (2, 3)]: + results = tester.run(test, times=times, concurrent=concurrent) + self.assertEqual(len(results), times) + for result in results.itervalues(): + self.assertEqual(result['status'], 0) def test_running_multiple_tests(self): tester = utils.Verifier(mock.MagicMock(), self.cloud_config_path) @@ -197,24 +195,27 @@ class VerifierTestCase(test.NoDBTestCase): 'test1': ['./tests/benchmark/test_utils.py', '-k', 'test_dummy_1'], 'test2': ['./tests/benchmark/test_utils.py', '-k', 'test_dummy_2'] } - for test_results in tester.run_all(tests_dict): - for result in test_results.itervalues(): - self.assertEqual(result['status'], 0) + with mock.patch('rally.benchmark.utils.fuel_cleanup.cleanup'): + for test_results in tester.run_all(tests_dict): + for result in test_results.itervalues(): + self.assertEqual(result['status'], 0) def test_tester_timeout(self): tester = utils.Verifier(mock.MagicMock(), self.cloud_config_path) test = ['./tests/benchmark/test_utils.py', '-k', 'test_dummy_timeout', '--timeout', '1'] - results = tester.run(test, times=2, concurrent=2) - for result in results.values(): - self.assertTrue('Timeout' in result['msg']) - self.assertTrue(result['status'] != 0) + with mock.patch('rally.benchmark.utils.fuel_cleanup.cleanup'): + results = tester.run(test, times=2, concurrent=2) + for result in results.values(): + self.assertTrue('Timeout' in result['msg']) + self.assertTrue(result['status'] != 0) def test_tester_no_timeout(self): tester = utils.Verifier(mock.MagicMock(), self.cloud_config_path) test = ['./tests/benchmark/test_utils.py', '-k', 'test_dummy_timeout', '--timeout', '2'] - results = tester.run(test, times=2, concurrent=2) - for result in results.values(): - self.assertTrue('Timeout' not in result['msg']) - self.assertTrue(result['status'] == 0) + with mock.patch('rally.benchmark.utils.fuel_cleanup.cleanup'): + results = tester.run(test, times=2, concurrent=2) + for result in results.values(): + self.assertTrue('Timeout' not in result['msg']) + self.assertTrue(result['status'] == 0)