tests: Fix bugs revealed by mock's recent change

Once the asserts were corrected, these tests started failing.
Fortunately, they didn't seem to be masking any actual bugs.

Signed-off-by: Zack Cerza <zack@redhat.com>

diff --git a/teuthology/orchestra/test/test_cluster.py b/teuthology/orchestra/test/test_cluster.py
index 8d320c49ae2e4bf00e402e471b503123b4835329..27bef8b831ae1c49911c1296d68d2ef11cfb21d4 100644 (file)
--- a/teuthology/orchestra/test/test_cluster.py
+++ b/teuthology/orchestra/test/test_cluster.py
@@ -66,9 +66,9 @@ class TestCluster(object):
                 (r2, ['baz']),
                 ],
             )
-        r1.run.assert_called_once_with(args=['test'])
-        r2.run.assert_called_once_with(args=['test'])
         got = c.run(args=['test'])
+        r1.run.assert_called_once_with(args=['test'], wait=True)
+        r2.run.assert_called_once_with(args=['test'], wait=True)
         assert len(got) == 2
         assert got, [ret1 == ret2]
         # check identity not equality

diff --git a/teuthology/orchestra/test/test_remote.py b/teuthology/orchestra/test/test_remote.py
index c11bdbf0444cca57bbc2906d20c63e791578aa07..76eae68bad892895d8edb7ff683df589dabf4fe8 100644 (file)
--- a/teuthology/orchestra/test/test_remote.py
+++ b/teuthology/orchestra/test/test_remote.py
@@ -61,7 +61,8 @@ class TestRemote(object):
         rem._runner = m_run
         result = rem.run(args=args)
         m_transport.getpeername.assert_called_once_with()
-        m_run.assert_called_once_with(args=args)
+        m_run_call_kwargs = m_run.call_args_list[0][1]
+        assert m_run_call_kwargs['args'] == args
         assert result is proc
         assert result.remote is rem
 
@@ -101,7 +102,7 @@ class TestRemote(object):
         stdout.seek(0)
         proc = RemoteProcess(
             client=self.m_ssh,
-            args='fakey',
+            args=args,
             )
         proc._stdout_buf = Mock()
         proc._stdout_buf.channel = Mock()
@@ -111,15 +112,12 @@ class TestRemote(object):
         m_run.return_value = proc
         r = remote.Remote(name='jdoe@xyzzy.example.com', ssh=self.m_ssh)
         r._runner = m_run
-        m_transport.getpeername.assert_called_once_with()
-        proc._stdout_buf.channel.recv_exit_status.assert_called_once_with()
-        m_run.assert_called_once_with(
-            client=self.m_ssh,
-            args=args,
-            stdout=BytesIO(),
-            name=r.shortname,
-        )
         assert r.arch == 'test_arch'
+        assert len(m_run.call_args_list) == 1
+        m_run_call_kwargs = m_run.call_args_list[0][1]
+        assert m_run_call_kwargs['client'] == self.m_ssh
+        assert m_run_call_kwargs['name'] == r.shortname
+        assert m_run_call_kwargs['args'] == ' '.join(args)
 
     def test_host_key(self):
         m_key = MagicMock()

diff --git a/teuthology/provision/test/test_fog.py b/teuthology/provision/test/test_fog.py
index f94291be28d714823c95cf3ace8a1972cab67214..34a51affd1ded7424e67645f6d8df44860b900a3 100644 (file)
--- a/teuthology/provision/test/test_fog.py
+++ b/teuthology/provision/test/test_fog.py
@@ -88,6 +88,7 @@ class TestFOG(object):
         self.mocks['m_Remote_machine_type'].return_value = 'type1'
         obj = self.klass('name.fqdn', 'type', '1.0')
         host_id = 99
+        task_id = 1234
         with patch.multiple(
             'teuthology.provision.fog.FOG',
             get_host_data=DEFAULT,
@@ -99,6 +100,7 @@ class TestFOG(object):
             _fix_hostname=DEFAULT,
         ) as local_mocks:
             local_mocks['get_host_data'].return_value = dict(id=host_id)
+            local_mocks['schedule_deploy_task'].return_value = task_id
             if not success:
                 local_mocks['wait_for_deploy_task'].side_effect = RuntimeError
                 with raises(RuntimeError):
@@ -108,7 +110,7 @@ class TestFOG(object):
             local_mocks['get_host_data'].assert_called_once_with()
             local_mocks['set_image'].assert_called_once_with(host_id)
             local_mocks['schedule_deploy_task'].assert_called_once_with(host_id)
-            local_mocks['wait_for_deploy_task'].assert_called_once_with()
+            local_mocks['wait_for_deploy_task'].assert_called_once_with(task_id)
             if success:
                 local_mocks['_wait_for_ready'].assert_called_once_with()
                 local_mocks['_fix_hostname'].assert_called_once_with()
@@ -200,7 +202,7 @@ class TestFOG(object):
             local_mocks['get_image_data'].return_value = dict(id='13')
             obj.set_image(host_id)
             local_mocks['do_request'].assert_called_once_with(
-                '/host/999', 'put', '{"imageID": "13"}',
+                '/host/999', method='PUT', data='{"imageID": 13}',
             )
 
     def test_schedule_deploy_task(self):
@@ -225,7 +227,7 @@ class TestFOG(object):
             local_mocks['get_deploy_tasks'].return_value = host_tasks
             obj = self.klass('name.fqdn', 'type', '1.0')
             result = obj.schedule_deploy_task(host_id)
-            local_mocks['get_deploy_tasks'].assert_called_once_with()
+            assert len(local_mocks['get_deploy_tasks'].call_args_list) == 2
         assert len(self.mocks['m_requests_Session_send'].call_args_list) == 3
         assert result == task_id
 

diff --git a/teuthology/suite/test/test_init.py b/teuthology/suite/test/test_init.py
index e477a25c1eed2f5229184b0a460b1796daa8a64c..6e91eefa10116dd8bb72125a38939e69ca820310 100644 (file)
--- a/teuthology/suite/test/test_init.py
+++ b/teuthology/suite/test/test_init.py
@@ -77,13 +77,14 @@ def test_wait_success(m_get_jobs, caplog):
 
     in_progress = deepcopy(results)
     assert 0 == suite.wait('name', 1, 'http://UPLOAD_URL')
-    assert m_get_jobs.called_with('name', fields=['job_id', 'status'])
+    m_get_jobs.assert_any_call('name', fields=['job_id', 'status'])
     assert 0 == len(in_progress)
     assert 'fail http://UPLOAD_URL/name/2' in caplog.text
 
+    m_get_jobs.reset_mock()
     in_progress = deepcopy(results)
     assert 0 == suite.wait('name', 1, None)
-    assert m_get_jobs.called_with('name', fields=['job_id', 'status'])
+    m_get_jobs.assert_any_call('name', fields=['job_id', 'status'])
     assert 0 == len(in_progress)
     assert 'fail http://URL2' in caplog.text
 

diff --git a/teuthology/suite/test/test_merge.py b/teuthology/suite/test/test_merge.py
index 3b6789b9317121816c5cbac3b4a86d0b49b6df76..82a0bb67b0cabaf028668e82ff78c9e34c1b5e0a 100644 (file)
--- a/teuthology/suite/test/test_merge.py
+++ b/teuthology/suite/test/test_merge.py
@@ -2,7 +2,6 @@ import logging
 from textwrap import dedent
 
 from mock import patch, MagicMock
-from unittest import TestCase
 
 from teuthology.suite import build_matrix
 from teuthology.suite.merge import config_merge
@@ -10,7 +9,7 @@ from teuthology.test.fake_fs import make_fake_fstools
 
 log = logging.getLogger(__name__)
 
-class TestMerge(TestCase):
+class TestMerge:
     patchpoints = [
         'os.path.exists',
         'os.listdir',
@@ -20,7 +19,6 @@ class TestMerge(TestCase):
     ]
 
     def setup_method(self):
-        log.debug("setUp")
         self.mocks = dict()
         self.patchers = dict()
         for ppoint in self.__class__.patchpoints:
@@ -61,12 +59,12 @@ class TestMerge(TestCase):
         self.start_patchers(fake_fs)
         try:
             result = build_matrix.build_matrix('d0_0')
-            self.assertEqual(len(result), 1)
+            assert 1 == len(result)
             configs = list(config_merge(result))
-            self.assertEqual(len(configs), 1)
+            assert 1 == len(configs)
             desc, frags, yaml = configs[0]
-            self.assertIn("top", yaml)
-            self.assertNotIn("foo", yaml)
+            assert "top" in yaml
+            assert "foo" not in yaml
         finally:
             self.stop_patchers()
 
@@ -93,13 +91,13 @@ class TestMerge(TestCase):
         self.start_patchers(fake_fs)
         try:
             result = build_matrix.build_matrix('d0_0')
-            self.assertEqual(len(result), 2)
+            assert 2 == len(result)
             configs = list(config_merge(result))
-            self.assertEqual(len(configs), 1)
+            assert 1 == len(configs)
             desc, frags, yaml = configs[0]
-            self.assertIn("top", yaml)
-            self.assertIn("baz", yaml)
-            self.assertNotIn("foo", yaml)
+            assert "top" in yaml
+            assert "baz" in yaml
+            assert "foo" not in yaml
         finally:
             self.stop_patchers()
 
@@ -132,13 +130,13 @@ class TestMerge(TestCase):
         self.start_patchers(fake_fs)
         try:
             result = build_matrix.build_matrix('d0_0')
-            self.assertEqual(len(result), 2)
+            assert 2 == len(result)
             configs = list(config_merge(result))
-            self.assertEqual(len(configs), 1)
+            assert 1 == len(configs)
             desc, frags, yaml = configs[0]
-            self.assertIn("top", yaml)
-            self.assertIn("baz", yaml)
-            self.assertNotIn("foo", yaml)
+            assert "top" in yaml
+            assert "baz" in yaml
+            assert "foo" not in yaml
         finally:
             self.stop_patchers()
 
@@ -159,12 +157,12 @@ class TestMerge(TestCase):
         self.start_patchers(fake_fs)
         try:
             result = build_matrix.build_matrix('d0_0')
-            self.assertEqual(len(result), 1)
+            assert 1 == len(result)
             configs = list(config_merge(result))
-            self.assertEqual(len(configs), 1)
+            assert 1 == len(configs)
             desc, frags, yaml = configs[0]
-            self.assertIn("test", yaml)
-            self.assertDictEqual(yaml["test"], {})
+            assert "test" in yaml
+            assert {} == yaml["test"]
         finally:
             self.stop_patchers()
 
@@ -176,6 +174,7 @@ class TestMerge(TestCase):
                 teuthology:
                   postmerge:
                     - |
+                      log.debug(_ENV)
                       log.debug("_ENV contains:")
                       for k,v in pairs(_ENV) do
                         log.debug("_ENV['%s'] = %s", tostring(k), tostring(v))
@@ -225,8 +224,8 @@ class TestMerge(TestCase):
         self.start_patchers(fake_fs)
         try:
             result = build_matrix.build_matrix('d0_0')
-            self.assertEqual(len(result), 1)
+            assert 1 == len(result)
             configs = list(config_merge(result))
-            self.assertEqual(len(configs), 1)
+            assert 1 == len(configs)
         finally:
             self.stop_patchers()

diff --git a/teuthology/test/task/test_console_log.py b/teuthology/test/task/test_console_log.py
index b7842824caa41bddcaf29d2aaa3d2056fa96f5da..e2ad6981ea7f6f9389d6a91617a402880ad6f73a 100644 (file)
--- a/teuthology/test/task/test_console_log.py
+++ b/teuthology/test/task/test_console_log.py
@@ -73,16 +73,20 @@ class TestConsoleLog(TestTask):
     def test_begin(self, m_pconsole):
         with self.klass(self.ctx, self.task_config) as task:
             assert len(task.processes) == len(self.ctx.cluster.remotes)
+            expected_log_paths = []
             for remote in task.cluster.remotes.keys():
-                dest_path = os.path.join(
-                    self.ctx.archive, '%s.log' % remote.shortname)
-                remote.console.spawn_sol_log.assert_called_once_with(
-                        dest_path=dest_path)
+                expected_log_paths.append(
+                    os.path.join(self.ctx.archive, 'console_logs', '%s.log' % remote.shortname)
+                )
+            assert len(m_pconsole().spawn_sol_log.call_args_list) == len(task.cluster.remotes)
+            got_log_paths = [c[0][0] for c in m_pconsole().spawn_sol_log.call_args_list]
+            assert got_log_paths == expected_log_paths
 
     @patch('teuthology.orchestra.console.PhysicalConsole')
     def test_end(self, m_pconsole):
-        with self.klass(self.ctx, self.task_config) as task:
+        m_proc = m_pconsole().spawn_sol_log.return_value
+        m_proc.poll.return_value = None
+        with self.klass(self.ctx, self.task_config):
             pass
-        for proc in task.processes.values():
-            proc.terminate.assert_called_once_with()
-            proc.kill.assert_called_once_with()
+        assert len(m_proc.terminate.call_args_list) == len(self.ctx.cluster.remotes)
+        assert len(m_proc.kill.call_args_list) == len(self.ctx.cluster.remotes)

diff --git a/teuthology/test/test_worker.py b/teuthology/test/test_worker.py
index f2c32984dea823d08a8b476ccd7814f88415a9d6..97a4d738d892cbfa80602a61ba18cb8ed7aae707 100644 (file)
--- a/teuthology/test/test_worker.py
+++ b/teuthology/test/test_worker.py
@@ -1,4 +1,3 @@
-import beanstalkc
 import os
 
 from unittest.mock import patch, Mock, MagicMock
@@ -188,11 +187,13 @@ class TestWorker(object):
         config = dict(
             name="the_name",
             job_id="1",
+            suite_sha1="suite_hash",
         )
         archive_dir = '/archive/dir'
         log_file_path = '/worker/log'
         m_fetch_teuthology.return_value = '/teuth/path'
         m_fetch_qa_suite.return_value = '/suite/path'
+        m_ls_remote.return_value = 'teuth_hash'
         m_isdir.return_value = True
         m_teuth_config.teuthology_path = None
         got_config, teuth_bin_path = worker.prep_job(
@@ -207,9 +208,9 @@ class TestWorker(object):
             config['job_id'],
         )
         assert got_config['teuthology_branch'] == 'main'
-        m_fetch_teuthology.assert_called_once_with_args(branch='main')
+        m_fetch_teuthology.assert_called_once_with(branch='main', commit='teuth_hash')
         assert teuth_bin_path == '/teuth/path/virtualenv/bin'
-        m_fetch_qa_suite.assert_called_once_with_args(branch='main')
+        m_fetch_qa_suite.assert_called_once_with('main', 'suite_hash')
         assert got_config['suite_path'] == '/suite/path'
 
     def build_fake_jobs(self, m_connection, m_job, job_bodies):
@@ -220,12 +221,11 @@ class TestWorker(object):
         And a list of basic job bodies, return a list of mocked Job objects
         """
         # Make sure instantiating m_job returns a new object each time
-        m_job.side_effect = lambda **kwargs: Mock(spec=beanstalkc.Job)
         jobs = []
         job_id = 0
         for job_body in job_bodies:
             job_id += 1
-            job = m_job(conn=m_connection, jid=job_id, body=job_body)
+            job = MagicMock(conn=m_connection, jid=job_id, body=job_body)
             job.jid = job_id
             job.body = job_body
             jobs.append(job)