diff --git a/.codecov.yml b/.codecov.yml new file mode 100644 index 00000000..023c59b9 --- /dev/null +++ b/.codecov.yml @@ -0,0 +1,5 @@ +coverage: + notify: + slack: + default: + url: "secret:p86vG6xzWKglCbWuYQtBSGejyP2W0udsB8gacjwScz8BnSZTcfbIi6nkmJQU+62KZW7P2ZfJujXWYrm0OwPfSKHx4hFPm9QR+/qZBliA3ENlTiwDzNqZo6MpvClJnPksptVkM38KrjWbkXD4psXEsU34hhGca6NNCrwK6+oP98Y=" diff --git a/.travis.yml b/.disable-travis.yml similarity index 100% rename from .travis.yml rename to .disable-travis.yml diff --git a/.github/workflows/test_odin_control.yml b/.github/workflows/test_odin_control.yml new file mode 100644 index 00000000..9a4cc9f7 --- /dev/null +++ b/.github/workflows/test_odin_control.yml @@ -0,0 +1,49 @@ +name: Test odin-control + +on: + - push + - pull_request + +jobs: + build_and_test: + runs-on: ubuntu-latest + strategy: + matrix: + python-version: [2.7, 3.6, 3.7, 3.8, 3.9] + + steps: + - uses: actions/checkout@v2 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install tox tox-gh-actions coverage + - name: Test with tox + run: tox + - name: Merge tox env specific coverage files + run: | + coverage combine + coverage xml + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v1 + with: + name: ${{ matrix.python-version }} + fail_ci_if_error: true + + notify: + if: ${{ always() }} + runs-on: ubuntu-latest + needs: build_and_test + steps: + - name: Slack Notification on completion + uses: rtCamp/action-slack-notify@v2 + env: + SLACK_CHANNEL: odin-control-notify + SLACK_COLOR: ${{ needs.build_and_test.result }} + SLACK_ICON: https://avatars.githubusercontent.com/odin-detector?size=48 + SLACK_TITLE: "odin-control CI tests completed: ${{ needs.build_and_test.result }}" + SLACK_USERNAME: odin-detector + SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK }} diff --git a/README.md b/README.md index 4cd5f15b..541367ea 100644 --- a/README.md +++ b/README.md @@ -1,9 +1,4 @@ -# odin -Prototype of ODIN framework for evaluation purposes only +# odin-control -[![Build Status](https://travis-ci.org/odin-detector/odin-control.svg)](https://travis-ci.org/odin-detector/odin-control) -[![Coverage Status](https://coveralls.io/repos/github/odin-detector/odin-control/badge.svg?branch=master)](https://coveralls.io/github/odin-detector/odin-control?branch=master) -[![Stories in Ready](https://badge.waffle.io/odin-detector/odin-control.png?label=ready&title=Ready)](https://waffle.io/odin-detector/odin-control) -[![Stories in In Progress](https://badge.waffle.io/odin-detector/odin-control.png?label=In%20Progress&title=In%20Progress)](https://waffle.io/odin-detector/odin-control) -[![Code Climate](https://codeclimate.com/github/odin-detector/odin-control/badges/gpa.svg)](https://codeclimate.com/github/odin-detector/odin-control) -[![Test Coverage](https://codeclimate.com/github/odin-detector/odin-control/badges/coverage.svg)](https://codeclimate.com/github/odin-detector/odin-control/coverage) +[![Test odin-control](https://github.com/odin-detector/odin-control/actions/workflows/test_odin_control.yml/badge.svg)](https://github.com/odin-detector/odin-control/actions/workflows/test_odin_control.yml) +[![codecov](https://codecov.io/gh/odin-detector/odin-control/branch/master/graph/badge.svg?token=Urucx8wsTU)](https://codecov.io/gh/odin-detector/odin-control) \ No newline at end of file diff --git a/setup.py b/setup.py index 0d442e67..aefc55c8 100644 --- a/setup.py +++ b/setup.py @@ -9,23 +9,34 @@ 'psutil>=5.0', ] +extras_require = { + 'test': [ + 'pytest', 'pytest-cov', 'requests', 'tox' + ] +} + if sys.version_info[0] == 2: install_requires.append('futures') + extras_require['test'].append('mock') +else: + extras_require['test'].append('pytest-asyncio') setup( - name="odin", + name="odin_control", version=versioneer.get_version(), cmdclass=versioneer.get_cmdclass(), - description='ODIN detector server', - url='https://github.com/timcnicholls/odin', + description='ODIN detector control system', + url='https://github.com/odin-detector/odin-control', author='Tim Nicholls', author_email='tim.nicholls@stfc.ac.uk', packages=find_packages('src'), - package_dir={'':'src'}, + package_dir={'': 'src'}, entry_points={ - 'console_scripts' : [ - 'odin_server = odin.server:main', + 'console_scripts': [ + 'odin_server = odin.server:main_deprecate', + 'odin_control = odin.server:main', ], }, install_requires=install_requires, + extras_require=extras_require, ) diff --git a/src/odin/server.py b/src/odin/server.py index 891e648d..96976a40 100644 --- a/src/odin/server.py +++ b/src/odin/server.py @@ -23,9 +23,9 @@ def shutdown_handler(): # pragma: no cover def main(argv=None): - """Run ODIN server. + """Run the odin-control server. - This function is the main entry point for the ODIN server. It parses configuration + This function is the main entry point for the odin-control server. It parses configuration options from the command line and any files, resolves adapters and launches the main API server before entering the IO processing loop. @@ -77,5 +77,26 @@ def main(argv=None): return 0 +def main_deprecate(argv=None): # pragma: no cover + """Deprecated main entry point for running the odin control server. + + This method adds an entry point for running odin control server that is run by the + deprecated odin_server command. It simply runs the main entry point as normal having + printing a deprecation warning. + """ + import warnings + with warnings.catch_warnings(): + warnings.simplefilter('always', DeprecationWarning) + message = """ + +The odin_server script entry point is deprecated and will be removed in future releases. Consider +using \'odin_control\' instead + + """ + warnings.warn(message, DeprecationWarning, stacklevel=1) + + main(argv) + + if __name__ == '__main__': # pragma: no cover sys.exit(main()) diff --git a/tests/adapters/test_proxy.py b/tests/adapters/test_proxy.py index 4d7603c8..e7998c06 100644 --- a/tests/adapters/test_proxy.py +++ b/tests/adapters/test_proxy.py @@ -20,7 +20,7 @@ from odin.adapters.parameter_tree import ParameterTree, ParameterTreeError from odin.adapters.adapter import wants_metadata from odin.util import convert_unicode_to_string -from tests.utils import LogCaptureFilter, log_message_seen +from tests.utils import log_message_seen if sys.version_info[0] == 3: # pragma: no cover from unittest.mock import Mock, patch diff --git a/tests/adapters/test_system_status.py b/tests/adapters/test_system_status.py index e4a8af59..553b1bb0 100644 --- a/tests/adapters/test_system_status.py +++ b/tests/adapters/test_system_status.py @@ -20,31 +20,83 @@ from tests.utils import log_message_seen + class SystemStatusTestFixture(): """Container class used in fixtures for testing the SystemStatus class.""" - def __init__(self): + def __init__(self, scoped_patcher): """Initialise a SystemStatus instance with an appropriate configuration.""" if platform.system() == 'Darwin': - self.lo_iface='lo0' + self.lo_iface = 'lo0' else: - self.lo_iface='lo' - - self.interfaces="{}, bad".format(self.lo_iface) + self.lo_iface = 'lo' + + self.interfaces = "{}, bad".format(self.lo_iface) self.disks = "/, /bad" - self.processes = "python, proc2" self.rate = 0.001 + self.mocked_proc_name = "mock_proc" + self.processes = ", ".join([self.mocked_proc_name, "proc2"]) + + self.mocked_procs = [] + self.parent_process = 1 + self.child_process = 0 + self.cpu_affinity_vals = [1, 2, 3] + + for idx, (name, cmdline) in enumerate( + [ + (self.mocked_proc_name, "one two"), + (self.mocked_proc_name, "four five"), + ("bash", self.mocked_proc_name + " etc"), + ] + ): + proc = Mock() + proc.pid = 1000 + idx + proc.name.return_value = name + proc.cmdline.return_value = cmdline.split() + proc.cpu_percent.return_value = 1.23 + if idx == self.parent_process: + proc.children.return_value = [self.mocked_procs[self.child_process]] + else: + proc.children.return_value = None + if idx > 0: + proc.cpu_affinity.return_value = self.cpu_affinity_vals + else: + delattr(proc, 'cpu_affinity') + proc.status.return_value = psutil.STATUS_RUNNING + + self.mocked_procs.append(proc) + + self.num_mocked_procs = len(self.mocked_procs) + + def mock_process_iter(attrs=None, ad_value=None): + + procs_to_yield = min(self.num_mocked_procs, len(self.mocked_procs)) + + for proc in self.mocked_procs[:procs_to_yield]: + yield proc + + scoped_patcher.setattr(psutil, "process_iter", mock_process_iter) + self.system_status = SystemStatus( interfaces=self.interfaces, disks=self.disks, processes=self.processes, rate=self.rate) -@pytest.fixture(scope="class") +#@pytest.fixture(scope="class") +@pytest.fixture() def test_system_status(): """Fixture used in SystemStatus test cases.""" - test_system_status = SystemStatusTestFixture() + + # Create a class-scoped monkey patcher to be used in the main fixture + from _pytest.monkeypatch import MonkeyPatch + scoped_patcher = MonkeyPatch() + + # Create the test fixture and yield to the tests + test_system_status = SystemStatusTestFixture(scoped_patcher) yield test_system_status + # Undo the patcher + scoped_patcher.undo() class TestSystemStatus(): """Test cases for the SystemStatus class.""" @@ -96,6 +148,7 @@ def test_system_status_monitor_processes(self, test_system_status): assert type(result) is dict def test_system_status_monitor(self, test_system_status): + test_system_status.num_mocked_procs = 2 """Test that monitoring the status of the system does not raise an exception.""" test_system_status.system_status.monitor() @@ -139,7 +192,7 @@ def test_default_rate_argument(self, test_system_status): Singleton._instances = {} temp_system_status = SystemStatus( interfaces=test_system_status.interfaces, - disks=test_system_status.disks, + disks=test_system_status.disks, processes=test_system_status.processes, ) assert pytest.approx(1.0) == temp_system_status._update_interval @@ -148,83 +201,93 @@ def test_default_rate_argument(self, test_system_status): def test_num_processes_change(self, test_system_status, caplog): """Test that monitoring processes correctly detects a change in the number of processes.""" - test_system_status.stash_method = test_system_status.system_status.find_processes - test_system_status.stash_processes = dict(test_system_status.system_status._processes) - test_system_status.system_status._processes = {} - test_system_status.system_status._processes['python'] = \ - test_system_status.stash_processes['python'] - current_processes = test_system_status.system_status.find_processes('python') - patched_processes = list(current_processes) - patched_processes.append(current_processes[0]) - - - test_system_status.system_status.find_processes = Mock(return_value = patched_processes) + # Ensure that the process monitoring has run once + test_system_status.system_status.monitor_processes() + # Reduce the number of processes to find and monitor again - + test_system_status.num_mocked_procs -= 1 logging.getLogger().setLevel(logging.DEBUG) test_system_status.system_status.monitor_processes() - # monitor_process will detect change in number of processes and log a debug message + # monitor_process will detect change in number of processes and log a debug message assert log_message_seen( - caplog, logging.DEBUG, "Number of processes named python is now") - - test_system_status.system_status.find_processes = test_system_status.stash_method - test_system_status.system_status._processes = test_system_status.stash_processes - + caplog, logging.DEBUG, "Number of processes named mock_proc is now") def test_find_processes_handles_children(self, test_system_status): """Test that process monitoring correctly handles child processes.""" - test_system_status.stash_method = test_system_status.system_status.find_processes_by_name - test_system_status.stash_processes = dict(test_system_status.system_status._processes) + mocked_procs = test_system_status.system_status._processes[ + test_system_status.mocked_proc_name] + assert mocked_procs[test_system_status.parent_process] \ + == mocked_procs[test_system_status.child_process] - current_processes = test_system_status.system_status.find_processes_by_name('python') - patched_processes = list(current_processes) - patched_processes[0].children = Mock(return_value = [patched_processes[-1]]) + def test_find_processes_matches_cmdline(self, test_system_status): + """Test that finding processes by name can match against the command line also.""" + num_procs_found = len(test_system_status.system_status.find_processes_by_name( + test_system_status.mocked_proc_name + )) + assert num_procs_found == test_system_status.num_mocked_procs - test_system_status.system_status.find_processes_by_name = Mock( - return_value = patched_processes) + def test_monitor_process_cpu_affinity(self, test_system_status, monkeypatch): + """Test that monitoring processes reports CPU affinity where implemented.""" test_system_status.system_status.monitor_processes() - - test_system_status.system_status.find_processes_by_name = test_system_status.stash_method - test_system_status.system_status._processes = test_system_status.stash_processes - - test_system_status.system_status.monitor_processes() - - - def test_monitor_process_cpu_affinity(self, test_system_status): - """Test that monitoring processes can cope with psutil reporting CPU affinity or not.""" - - test_system_status.stash_proc = test_system_status.system_status._processes['python'][0] - - setattr(test_system_status.system_status._processes['python'][0], 'cpu_affinity', lambda: [1,2,3]) + cpu_affinity_vals = [status['cpu_affinity'] for status in + test_system_status.system_status._process_status[ + test_system_status.mocked_proc_name + ].values() + ] + assert test_system_status.cpu_affinity_vals in cpu_affinity_vals + + def test_monitor_process_no_cpu_affinity(self, test_system_status, monkeypatch): + """Test that monitoring processes handles systems without CPU affinity support.""" + try: + monkeypatch.delattr(psutil.Process, 'cpu_affinity') + except AttributeError: + pass test_system_status.system_status.monitor_processes() - delattr(test_system_status.system_status._processes['python'][0], 'cpu_affinity') test_system_status.system_status.monitor_processes() - - test_system_status.system_status._processes['python'][0] = test_system_status.stash_proc + cpu_affinity_vals = [status['cpu_affinity'] for status in + test_system_status.system_status._process_status[ + test_system_status.mocked_proc_name + ].values() + ] + assert None in cpu_affinity_vals def test_monitor_process_traps_nosuchprocess(self, test_system_status): """Test that monitoring processes can cope with processing disappearing.""" - with patch('psutil.Process.memory_info', spec=True) as mocked: - mocked.side_effect = psutil.NoSuchProcess('') - test_system_status.system_status.monitor_processes() + test_system_status.mocked_procs[0].memory_info.side_effect = psutil.NoSuchProcess('') + test_system_status.system_status.monitor_processes() def test_monitor_process_traps_accessdenied(self, test_system_status): """Test that monitoring processes can cope with being denied access to process info.""" - with patch('psutil.Process.memory_info', spec=True) as mocked: - mocked.side_effect = psutil.AccessDenied('') - test_system_status.system_status.monitor_processes() - + test_system_status.mocked_procs[0].memory_info.side_effect = psutil.AccessDenied('') + test_system_status.system_status.monitor_processes() + def test_find_processes_traps_accessdenied(self, test_system_status): """Test that finding processes can cope with being denied access to process info.""" - with patch('psutil.Process.cpu_percent', spec=True) as mocked: - mocked.side_effect = psutil.AccessDenied('') - processes = test_system_status.system_status.find_processes('python') - # If all processes are AccessDenied then the returned list will be empty - assert not processes - + for proc in test_system_status.mocked_procs: + proc.cpu_percent.side_effect = psutil.AccessDenied('') + processes = test_system_status.system_status.find_processes( + test_system_status.mocked_proc_name + ) + # If all processes are AccessDenied then the returned list will be empty + assert not processes + + @pytest.mark.parametrize( + "test_exc", [psutil.AccessDenied, psutil.ZombieProcess, psutil.NoSuchProcess] + ) + def test_find_processes_by_name_traps_exceptions(self, test_system_status, test_exc): + """Test the finding processes by name traps various psutil exception cases.""" + for proc in test_system_status.mocked_procs: + proc.name.side_effect = test_exc('') + processes = test_system_status.system_status.find_processes_by_name( + test_system_status.mocked_proc_name + ) + # If all process lookups result in an exception, the returned list will be empty + assert not processes + class SystemStatusAdapterTestFixture(): """Container class used in fixtures for testing SystemStatusAdapter.""" @@ -243,6 +306,7 @@ def test_sysstatus_adapter(): test_sysstatus_adapter = SystemStatusAdapterTestFixture() yield test_sysstatus_adapter + class TestSystemStatusAdapter(): """Test cases for the SystemStatusAdapter class.""" @@ -273,7 +337,7 @@ def test_adapter_put(self, test_sysstatus_adapter): response = test_sysstatus_adapter.adapter.put( test_sysstatus_adapter.path, test_sysstatus_adapter.request) - + assert response.data == expected_response assert response.status_code == 200 diff --git a/tests/routes/test_default.py b/tests/routes/test_default.py index 8096dfee..fe76a198 100644 --- a/tests/routes/test_default.py +++ b/tests/routes/test_default.py @@ -9,7 +9,7 @@ import pytest from odin.http.routes.default import DefaultRoute -from tests.utils import LogCaptureFilter +from tests.utils import log_message_seen class TestDefaultRoute(): """Test DefaultRoute class.""" @@ -33,10 +33,5 @@ def test_default_route_bad_path(self, caplog): def_route = DefaultRoute(path) assert path in def_route.default_handler_args['path'] - msg_seen = False - for record in caplog.records: - if (record.levelno == logging.WARNING and - 'Default handler static path does not exist' in record.getMessage()): - msg_seen = True - - assert msg_seen \ No newline at end of file + assert log_message_seen(caplog, logging.WARNING, + 'Default handler static path does not exist') \ No newline at end of file diff --git a/tests/test_server.py b/tests/test_server.py index 55a37400..374bfdaf 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -8,11 +8,11 @@ from unittest.mock import Mock else: # pragma: no cover from mock import Mock - + from odin.http.server import HttpServer from odin import server -from tests.utils import OdinTestServer, LogCaptureFilter +from tests.utils import OdinTestServer, log_message_seen @pytest.fixture(scope="class") def odin_test_server(): @@ -152,18 +152,13 @@ def test_server_entry_config_error(self): class TestOdinServerAccessLogging(): """Class for testing a bad access logging level congiguration.""" - def test_bad_access_log_level(self): + def test_bad_access_log_level(self, caplog): """Test that a bad access logging level generates an error.""" - log_capture_filter = LogCaptureFilter() bad_level='wibble' http_server = HttpServer(adapters=[], access_logging=bad_level) - - msg_seen = False - expected_msg = 'Access logging level {} not recognised'.format(bad_level) - for msg in log_capture_filter.log_error(): - if msg == expected_msg: - msg_seen = True - assert msg_seen + + assert log_message_seen(caplog, logging.ERROR, + 'Access logging level {} not recognised'.format(bad_level)) @pytest.fixture(scope="class") def no_adapter_server(): @@ -175,14 +170,11 @@ def no_adapter_server(): class TestOdinServerMissingAdapters(object): """Class to test a server with no adapters loaded.""" - def test_server_missing_adapters(self, no_adapter_server): + def test_server_missing_adapters(self, no_adapter_server, caplog): """Test that a server with no adapters loaded generates a warning message.""" - no_adapters_msg_seen = False - for msg in no_adapter_server.log_capture_filter.log_warning(): - if msg == 'Failed to resolve API adapters: No adapters specified in configuration': - no_adapters_msg_seen = True - - assert no_adapters_msg_seen + assert log_message_seen(caplog, logging.WARNING, + 'Failed to resolve API adapters: No adapters specified in configuration', + when="setup") class MockHandler(object): """Class for mocking tornado request handler objects.""" diff --git a/tests/utils.py b/tests/utils.py index 5e1c7a9e..3ec48009 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -22,38 +22,9 @@ from odin import server +def log_message_seen(caplog, level, message, when="call"): -class LogCaptureFilter(logging.Filter): - - def __init__(self, *args, **kwargs): - - logging.Filter.__init__(self, *args, **kwargs) - self.messages = {logging.DEBUG: [], - logging.INFO: [], - logging.WARNING: [], - logging.ERROR: [], - logging.CRITICAL: [] - } - - root_logger = logging.getLogger() - if len(root_logger.handlers) == 0: - root_logger.addHandler(logging.handlers.MemoryHandler(100)) # pragma: nocover - - root_logger.handlers[0].addFilter(self) - - for level in self.messages: - msg_getter_name = 'log_{}'.format(logging.getLevelName(level).lower()) - setattr(self, msg_getter_name, lambda self=self, level=level: self.messages[level]) - - def filter(self, record): - - self.messages[record.levelno].append(record.getMessage()) - return True - - -def log_message_seen(caplog, level, message): - - for record in caplog.records: + for record in caplog.get_records(when): if record.levelno == level and message in record.getMessage(): return True @@ -70,7 +41,6 @@ def __init__(self, server_port=server_port, adapter_config=None, access_logging= self.server_thread = None self.server_event_loop = None - self.log_capture_filter = None self.server_conf_file = NamedTemporaryFile(mode='w+') parser = ConfigParser() @@ -82,7 +52,7 @@ def __init__(self, server_port=server_port, adapter_config=None, access_logging= parser.set('server', 'debug_mode', '1') parser.set('server', 'http_port', str(server_port)) parser.set('server', 'http_addr', self.server_addr) - parser.set('server', 'static_path', static_path) + parser.set('server', 'static_path', static_path) if adapter_config is not None: adapters = ', '.join([adapter for adapter in adapter_config]) @@ -104,8 +74,6 @@ def __init__(self, server_port=server_port, adapter_config=None, access_logging= parser.write(self.server_conf_file) self.server_conf_file.file.flush() - self.log_capture_filter = LogCaptureFilter() - server_args = ['--config={}'.format(self.server_conf_file.name)] self.server_thread = threading.Thread(target=self._run_server, args=(server_args,)) self.server_thread.start() diff --git a/tox.ini b/tox.ini index 435cf19f..bb50c8c3 100644 --- a/tox.ini +++ b/tox.ini @@ -4,12 +4,15 @@ # and then run "tox" from this directory. [tox] -envlist = clean,py27-tornado{4,5},py37-tornado{5,6},report +envlist = clean,py27-tornado{4,5},py{36,37,38,39}-tornado{5,6},report -[travis] +[gh-actions] python = - 2.7: py27 - 3.7: py37, clean, report + 2.7: py27 + 3.6: py36 + 3.7: py37 + 3.8: py38 + 3.9: py39 [testenv] deps = @@ -17,16 +20,17 @@ deps = pytest-cov requests py27: mock + py{36,37,38,39}: pytest-asyncio tornado4: tornado>=4.0,<5.0 tornado5: tornado>=5.0,<6.0 tornado6: tornado>=6.0 -setenv = - {py27,py37}: COVERAGE_FILE=.coverage.{envname} +setenv = + py{27,36,37,38,39}: COVERAGE_FILE=.coverage.{envname} commands = - pytest --cov=odin {posargs:-vv} + pytest --cov=odin --cov-report=term-missing {posargs:-vv} depends = - {py27,py37}: clean - report: py27,py37 + py{27,36,37,38,39}: clean + report: py{27,36,37,38,39} [testenv:clean] skip_install = true @@ -37,5 +41,5 @@ commands = coverage erase skip_install = true deps = coverage commands = - coverage combine + coverage combine coverage report -m