From 1071ae6a211f01978736c9a9c9b2db73cd15317c Mon Sep 17 00:00:00 2001 From: Sebastiaan Huber Date: Wed, 22 Sep 2021 14:07:06 +0200 Subject: [PATCH] `verdi process list`: fix logging bug in `check_worker_load` The `check_worker_load` utility, called in `verdi process list`, caused an exception in the logging because it logged a message including a literal percentage sign. However, percentage signs carry special meaning as they are placeholders for argument substitution and literal percentage signs need to be escaped by doubling the character. --- aiida/cmdline/utils/common.py | 21 ++-- tests/cmdline/utils/test_common.py | 172 ++++++++++++++++------------- 2 files changed, 106 insertions(+), 87 deletions(-) diff --git a/aiida/cmdline/utils/common.py b/aiida/cmdline/utils/common.py index eb1f84ffe4..36fa393170 100644 --- a/aiida/cmdline/utils/common.py +++ b/aiida/cmdline/utils/common.py @@ -491,12 +491,13 @@ def get_num_workers(): def check_worker_load(active_slots): - """ - Check if the percentage usage of the daemon worker slots exceeds a threshold. - If it does, print a warning. + """Log a message with information on the current daemon worker load. + + If there are daemon workers active, it logs the current load. If that exceeds 90%, a warning is included with the + suggestion to run ``verdi daemon incr``. - The purpose of this check is to warn the user if they are close to running out of worker slots - which could lead to their processes becoming stuck indefinitely. + The purpose of this check is to warn the user if they are close to running out of worker slots which could lead to + their processes becoming stuck indefinitely. :param active_slots: the number of currently active worker slots """ @@ -511,16 +512,16 @@ def check_worker_load(active_slots): try: active_workers = get_num_workers() except CircusCallError: - echo.echo_critical('Could not contact Circus to get the number of active workers') + echo.echo_critical('Could not contact Circus to get the number of active workers.') if active_workers is not None: available_slots = active_workers * slots_per_worker percent_load = 1.0 if not available_slots else (active_slots / available_slots) if percent_load > warning_threshold: echo.echo('') # New line - echo.echo_warning(f'{percent_load * 100:.0f}% of the available daemon worker slots have been used!') - echo.echo_warning("Increase the number of workers with 'verdi daemon incr'.\n") + echo.echo_warning(f'{percent_load * 100:.0f}%% of the available daemon worker slots have been used!') + echo.echo_warning('Increase the number of workers with `verdi daemon incr`.') else: - echo.echo_report(f'Using {percent_load * 100:.0f}% of the available daemon worker slots') + echo.echo_report(f'Using {percent_load * 100:.0f}%% of the available daemon worker slots.') else: - echo.echo_report('No active daemon workers') + echo.echo_report('No active daemon workers.') diff --git a/tests/cmdline/utils/test_common.py b/tests/cmdline/utils/test_common.py index 4447a5c126..0df23c4309 100644 --- a/tests/cmdline/utils/test_common.py +++ b/tests/cmdline/utils/test_common.py @@ -7,106 +7,124 @@ # For further information on the license, see the LICENSE.txt file # # For further information please visit http://www.aiida.net # ########################################################################### -"""Tests for common command line utilities.""" +"""Tests for the :mod:`aiida.cmdline.utils.common` module.""" +import pytest + +from aiida.cmdline.utils import common +from aiida.common import LinkType +from aiida.engine import Process, calcfunction +from aiida.orm import CalcFunctionNode, CalculationNode, WorkflowNode + + +@pytest.mark.usefixtures('clear_database_before_test') +def test_get_node_summary(aiida_local_code_factory): + """Test the ``get_node_summary`` utility.""" + code = aiida_local_code_factory(entry_point='core.arithmetic.add', executable='/bin/bash') + node = CalculationNode() + node.computer = code.computer + node.add_incoming(code, link_type=LinkType.INPUT_CALC, link_label='code') + node.store() -from aiida import orm -from aiida.backends.testbase import AiidaTestCase -from aiida.common.links import LinkType + summary = common.get_node_summary(node) + assert node.uuid in summary + assert node.computer.label in summary -class TestCommonUtilities(AiidaTestCase): - """Tests for common command line utilities.""" +@pytest.mark.usefixtures('clear_database_before_test') +def test_get_node_info_multiple_call_links(): + """Test the ``get_node_info`` utility. - def test_get_node_summary(self): - """Test the `get_node_summary` utility.""" - from aiida.cmdline.utils.common import get_node_summary + Regression test for #2868: + Verify that all `CALL` links are included in the formatted string even if link labels are identical. + """ + workflow = WorkflowNode().store() + node_one = CalculationNode() + node_two = CalculationNode() - computer_label = self.computer.label # pylint: disable=no-member + node_one.add_incoming(workflow, link_type=LinkType.CALL_CALC, link_label='CALL_IDENTICAL') + node_two.add_incoming(workflow, link_type=LinkType.CALL_CALC, link_label='CALL_IDENTICAL') + node_one.store() + node_two.store() - code = orm.Code( - input_plugin_name='core.arithmetic.add', - remote_computer_exec=[self.computer, '/remote/abs/path'], - ) - code.store() + node_info = common.get_node_info(workflow) + assert 'CALL_IDENTICAL' in node_info + assert str(node_one.pk) in node_info + assert str(node_two.pk) in node_info - node = orm.CalculationNode() - node.computer = self.computer - node.add_incoming(code, link_type=LinkType.INPUT_CALC, link_label='code') - node.store() - summary = get_node_summary(node) - self.assertIn(node.uuid, summary) - self.assertIn(computer_label, summary) +@pytest.mark.usefixtures('clear_database_before_test') +def test_get_process_function_report(): + """Test the ``get_process_function_report`` utility.""" + warning = 'You have been warned' + node = CalcFunctionNode() + node.store() - def test_get_node_info_multiple_call_links(self): - """Test the `get_node_info` utility. + node.logger.warning(warning) + assert warning in common.get_process_function_report(node) - Regression test for #2868: - Verify that all `CALL` links are included in the formatted string even if link labels are identical. - """ - from aiida.cmdline.utils.common import get_node_info - workflow = orm.WorkflowNode().store() - node_one = orm.CalculationNode() - node_two = orm.CalculationNode() +def test_print_process_info(): + """Test the ``print_process_info`` method.""" - node_one.add_incoming(workflow, link_type=LinkType.CALL_CALC, link_label='CALL_IDENTICAL') - node_two.add_incoming(workflow, link_type=LinkType.CALL_CALC, link_label='CALL_IDENTICAL') - node_one.store() - node_two.store() + class TestProcessWithoutDocstring(Process): + # pylint: disable=missing-docstring - node_info = get_node_info(workflow) - self.assertTrue('CALL_IDENTICAL' in node_info) - self.assertTrue(str(node_one.pk) in node_info) - self.assertTrue(str(node_two.pk) in node_info) + @classmethod + def define(cls, spec): + super().define(spec) + spec.input('some_input') - def test_get_process_function_report(self): - """Test the `get_process_function_report` utility.""" - from aiida.cmdline.utils.common import get_process_function_report + class TestProcessWithDocstring(Process): + """Some docstring.""" - warning = 'You have been warned' + @classmethod + def define(cls, spec): + super().define(spec) + spec.input('some_input') - node = orm.CalcFunctionNode() - node.store() + @calcfunction + def test_without_docstring(): + pass - # Add a log message through the logger - node.logger.warning(warning) + @calcfunction + def test_with_docstring(): + """Some docstring.""" - self.assertIn(warning, get_process_function_report(node)) + # We are just checking that the command does not except + common.print_process_info(TestProcessWithoutDocstring) + common.print_process_info(TestProcessWithDocstring) + common.print_process_info(test_without_docstring) + common.print_process_info(test_with_docstring) - def test_print_process_info(self): - """Test the `print_process_info` method.""" - from aiida.cmdline.utils.common import print_process_info - from aiida.common.utils import Capturing - from aiida.engine import Process, calcfunction - class TestProcessWithoutDocstring(Process): - # pylint: disable=missing-docstring +@pytest.mark.parametrize( + 'active_workers, active_slots, expected', ( + (None, None, 'No active daemon workers.'), + (1, 0, 'Report: Using 0% of the available daemon worker slots.'), + (1, 200, 'Warning: 100% of the available daemon worker slots have been used!'), + ) +) +def test_check_worker_load(monkeypatch, capsys, active_workers, active_slots, expected): + """Test the ``check_worker_load`` function. - @classmethod - def define(cls, spec): - super().define(spec) - spec.input('some_input') + We monkeypatch the ``get_num_workers`` method which is called by ``check_worker_load`` to return the number of + active workers that we parametrize. + """ + monkeypatch.setattr(common, 'get_num_workers', lambda: active_workers) + common.check_worker_load(active_slots) + assert expected in capsys.readouterr().out - class TestProcessWithDocstring(Process): - """Some docstring.""" - @classmethod - def define(cls, spec): - super().define(spec) - spec.input('some_input') +def test_check_worker_load_fail(monkeypatch, capsys): + """Test the ``check_worker_load`` function when ``get_num_workers`` will except with ``CircusCallError``.""" - @calcfunction - def test_without_docstring(): - pass + def get_num_workers(): + from aiida.common.exceptions import CircusCallError + raise CircusCallError - @calcfunction - def test_with_docstring(): - """Some docstring.""" + monkeypatch.setattr(common, 'get_num_workers', get_num_workers) - # We are just checking that the command does not except - with Capturing(): - print_process_info(TestProcessWithoutDocstring) - print_process_info(TestProcessWithDocstring) - print_process_info(test_without_docstring) - print_process_info(test_with_docstring) + with pytest.raises(SystemExit): + common.check_worker_load(None) + + assert 'Could not contact Circus to get the number of active workers.' in capsys.readouterr().err