From 0b920d2a08bc1b5b6447e9368329d124258e2a8c Mon Sep 17 00:00:00 2001 From: Riccardo Pittau Date: Mon, 8 Mar 2021 18:53:49 +0100 Subject: [PATCH] Enforce autospec in test_console_utils Adjust unit tests and filters in tox.ini Change-Id: I05a3a1b37d0ce3cf33b3b5b9637350f0fd1f0afb --- .../drivers/modules/test_console_utils.py | 81 ++++++++++++++----- tox.ini | 4 +- 2 files changed, 61 insertions(+), 24 deletions(-) diff --git a/ironic/tests/unit/drivers/modules/test_console_utils.py b/ironic/tests/unit/drivers/modules/test_console_utils.py index 3419abb4a2..4ce3ae4113 100644 --- a/ironic/tests/unit/drivers/modules/test_console_utils.py +++ b/ironic/tests/unit/drivers/modules/test_console_utils.py @@ -56,6 +56,15 @@ class ConsoleUtilsTestCase(db_base.DbTestCase): self.context, driver_info=INFO_DICT) self.info = ipmi._parse_driver_info(self.node) + self.mock_stdout = tempfile.NamedTemporaryFile(delete=False) + self.mock_stderr = tempfile.NamedTemporaryFile(delete=False) + + def tearDown(self): + super(ConsoleUtilsTestCase, self).tearDown() + self.mock_stdout.close() + self.mock_stderr.close() + os.remove(self.mock_stdout.name) + os.remove(self.mock_stderr.name) def test__get_console_pid_dir(self): pid_dir = '/tmp/pid_dir' @@ -269,7 +278,7 @@ class ConsoleUtilsTestCase(db_base.DbTestCase): @mock.patch.object(console_utils, 'open', mock.mock_open(read_data='12345\n')) @mock.patch.object(os.path, 'exists', autospec=True) - @mock.patch.object(subprocess, 'Popen') + @mock.patch.object(subprocess, 'Popen', autospec=True) @mock.patch.object(psutil, 'pid_exists', autospec=True) @mock.patch.object(console_utils, '_ensure_console_pid_dir_exists', autospec=True) @@ -280,8 +289,15 @@ class ConsoleUtilsTestCase(db_base.DbTestCase): mock_popen, mock_path_exists, mock_fcntl): mock_popen.return_value.poll.return_value = 0 - mock_popen.return_value.stdout.return_value.fileno.return_value = 0 - mock_popen.return_value.stderr.return_value.fileno.return_value = 1 + + self.mock_stdout.write(b'0') + self.mock_stdout.seek(0) + mock_popen.return_value.stdout = self.mock_stdout + + self.mock_stderr.write(b'1') + self.mock_stderr.seek(0) + mock_popen.return_value.stderr = self.mock_stderr + mock_pid_exists.return_value = True mock_path_exists.return_value = True @@ -301,7 +317,7 @@ class ConsoleUtilsTestCase(db_base.DbTestCase): @mock.patch.object(console_utils, 'open', mock.mock_open(read_data='12345\n')) @mock.patch.object(os.path, 'exists', autospec=True) - @mock.patch.object(subprocess, 'Popen') + @mock.patch.object(subprocess, 'Popen', autospec=True) @mock.patch.object(psutil, 'pid_exists', autospec=True) @mock.patch.object(console_utils, '_ensure_console_pid_dir_exists', autospec=True) @@ -314,8 +330,15 @@ class ConsoleUtilsTestCase(db_base.DbTestCase): # no existing PID file before starting mock_stop.side_effect = exception.NoConsolePid('/tmp/blah') mock_popen.return_value.poll.return_value = 0 - mock_popen.return_value.stdout.return_value.fileno.return_value = 0 - mock_popen.return_value.stderr.return_value.fileno.return_value = 1 + + self.mock_stdout.write(b'0') + self.mock_stdout.seek(0) + mock_popen.return_value.stdout = self.mock_stdout + + self.mock_stderr.write(b'1') + self.mock_stderr.seek(0) + mock_popen.return_value.stderr = self.mock_stderr + mock_pid_exists.return_value = True mock_path_exists.return_value = True @@ -334,7 +357,7 @@ class ConsoleUtilsTestCase(db_base.DbTestCase): @mock.patch.object(time, 'sleep', autospec=True) @mock.patch.object(os, 'read', autospec=True) @mock.patch.object(fcntl, 'fcntl', autospec=True) - @mock.patch.object(subprocess, 'Popen') + @mock.patch.object(subprocess, 'Popen', autospec=True) @mock.patch.object(console_utils, '_ensure_console_pid_dir_exists', autospec=True) @mock.patch.object(console_utils, '_stop_console', autospec=True) @@ -342,10 +365,15 @@ class ConsoleUtilsTestCase(db_base.DbTestCase): self, mock_stop, mock_dir_exists, mock_popen, mock_fcntl, mock_os_read, mock_sleep): mock_popen.return_value.poll.return_value = 1 - stdout = mock_popen.return_value.stdout - stderr = mock_popen.return_value.stderr - stdout.return_value.fileno.return_value = 0 - stderr.return_value.fileno.return_value = 1 + + self.mock_stdout.write(b'0') + self.mock_stdout.seek(0) + mock_popen.return_value.stdout = self.mock_stdout + + self.mock_stderr.write(b'1') + self.mock_stderr.seek(0) + mock_popen.return_value.stderr = self.mock_stderr + err_output = b'error output' mock_os_read.side_effect = [err_output] * 2 + [OSError] * 2 mock_fcntl.side_effect = [1, mock.Mock()] * 2 @@ -358,7 +386,7 @@ class ConsoleUtilsTestCase(db_base.DbTestCase): mock_stop.assert_called_once_with(self.info['uuid']) mock_sleep.assert_has_calls([mock.call(1), mock.call(1)]) mock_dir_exists.assert_called_once_with() - for obj in (stdout, stderr): + for obj in (self.mock_stdout, self.mock_stderr): mock_fcntl.assert_has_calls([ mock.call(obj, fcntl.F_GETFL), mock.call(obj, fcntl.F_SETFL, 1 | os.O_NONBLOCK)]) @@ -368,7 +396,7 @@ class ConsoleUtilsTestCase(db_base.DbTestCase): mock_popen.return_value.poll.assert_called_with() @mock.patch.object(fcntl, 'fcntl', autospec=True) - @mock.patch.object(subprocess, 'Popen') + @mock.patch.object(subprocess, 'Popen', autospec=True) @mock.patch.object(console_utils, '_ensure_console_pid_dir_exists', autospec=True) @mock.patch.object(console_utils, '_stop_console', autospec=True) @@ -377,8 +405,14 @@ class ConsoleUtilsTestCase(db_base.DbTestCase): self.config(subprocess_timeout=0, group='console') self.config(subprocess_checking_interval=0, group='console') mock_popen.return_value.poll.return_value = None - mock_popen.return_value.stdout.return_value.fileno.return_value = 0 - mock_popen.return_value.stderr.return_value.fileno.return_value = 1 + + self.mock_stdout.write(b'0') + self.mock_stdout.seek(0) + mock_popen.return_value.stdout = self.mock_stdout + + self.mock_stderr.write(b'1') + self.mock_stderr.seek(0) + mock_popen.return_value.stderr = self.mock_stderr self.assertRaisesRegex( exception.ConsoleSubprocessFailed, 'Timeout or error', @@ -399,7 +433,7 @@ class ConsoleUtilsTestCase(db_base.DbTestCase): @mock.patch.object(console_utils, 'open', mock.mock_open(read_data='12345\n')) @mock.patch.object(os.path, 'exists', autospec=True) - @mock.patch.object(subprocess, 'Popen') + @mock.patch.object(subprocess, 'Popen', autospec=True) @mock.patch.object(psutil, 'pid_exists', autospec=True) @mock.patch.object(console_utils, '_ensure_console_pid_dir_exists', autospec=True) @@ -408,10 +442,15 @@ class ConsoleUtilsTestCase(db_base.DbTestCase): self, mock_stop, mock_dir_exists, mock_pid_exists, mock_popen, mock_path_exists, mock_fcntl, mock_os_read, mock_sleep): mock_popen.return_value.poll.return_value = 0 - stdout = mock_popen.return_value.stdout - stderr = mock_popen.return_value.stderr - stdout.return_value.fileno.return_value = 0 - stderr.return_value.fileno.return_value = 1 + + self.mock_stdout.write(b'0') + self.mock_stdout.seek(0) + mock_popen.return_value.stdout = self.mock_stdout + + self.mock_stderr.write(b'1') + self.mock_stderr.seek(0) + mock_popen.return_value.stderr = self.mock_stderr + mock_pid_exists.return_value = False mock_os_read.side_effect = [b'error output'] * 2 + [OSError] * 2 mock_fcntl.side_effect = [1, mock.Mock()] * 2 @@ -426,7 +465,7 @@ class ConsoleUtilsTestCase(db_base.DbTestCase): mock_stop.assert_called_once_with(self.info['uuid']) mock_sleep.assert_has_calls([mock.call(1), mock.call(1)]) mock_dir_exists.assert_called_once_with() - for obj in (stdout, stderr): + for obj in (self.mock_stdout, self.mock_stderr): mock_fcntl.assert_has_calls([ mock.call(obj, fcntl.F_GETFL), mock.call(obj, fcntl.F_SETFL, 1 | os.O_NONBLOCK)]) diff --git a/tox.ini b/tox.ini index 420e4daacd..e00496f5a3 100644 --- a/tox.ini +++ b/tox.ini @@ -135,12 +135,10 @@ max-complexity=18 # [H210] Require ‘autospec’, ‘spec’, or ‘spec_set’ in mock.patch/mock.patch.object calls # [H904] Delay string interpolations at logging calls. enable-extensions=H106,H203,H204,H205,H210,H904 -# TODO(rpittau) remove the ignores below when we're ready to apply H210 to -# the various modules. This can be done in batches changing the filters. +# [E402] Module level import not at top of file per-file-ignores = ironic/cmd/__init__.py:E402 ironic/tests/base.py:E402 - ironic/tests/unit/drivers/modules/test_console_utils.py:H210 [hacking] import_exceptions = testtools.matchers, ironic.common.i18n