From c70b4cd023cdedb82e51b0cc494abdc5e76eaec8 Mon Sep 17 00:00:00 2001 From: Bernhard Kaindl Date: Thu, 1 Feb 2024 12:00:00 +0000 Subject: [PATCH 1/2] rrdd.py: Python3: Fix crash on failure contacting xcp-rrdd Signed-off-by: Bernhard Kaindl --- ocaml/xcp-rrdd/scripts/rrdd/rrdd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ocaml/xcp-rrdd/scripts/rrdd/rrdd.py b/ocaml/xcp-rrdd/scripts/rrdd/rrdd.py index 909e9ab20c0..6cb7540dcdf 100644 --- a/ocaml/xcp-rrdd/scripts/rrdd/rrdd.py +++ b/ocaml/xcp-rrdd/scripts/rrdd/rrdd.py @@ -288,7 +288,7 @@ def wait_until_next_reading(self, neg_shift=1): return except socket.error: msg = "Failed to contact xcp-rrdd. Sleeping for 5 seconds .." - print >> sys.stderr, msg + print(msg, file=sys.stderr) time.sleep(5) def update(self): From 918a8a624956404a5aac6487c8af4e9d374dd2cb Mon Sep 17 00:00:00 2001 From: Bernhard Kaindl Date: Thu, 1 Feb 2024 12:00:00 +0000 Subject: [PATCH 2/2] CI: Unit-Test the crash-fix for rrdd.API.wait_until_next_reading() Signed-off-by: Bernhard Kaindl --- .github/workflows/main.yml | 8 +- ocaml/xcp-rrdd/scripts/rrdd/rrdd.py | 6 +- .../rrdd/test_api_wait_until_next_reading.py | 79 +++++++++++++++++++ 3 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 ocaml/xcp-rrdd/scripts/rrdd/test_api_wait_until_next_reading.py diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 8624dc4dc21..cadf84c35c4 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -54,11 +54,13 @@ jobs: run: pip install pandas pytype toml - name: Install common dependencies for Python ${{matrix.python-version}} - run: pip install mock pytest-coverage pytest-mock + run: pip install future mock pytest-coverage pytest-mock - - name: Run Pytest tests for Python ${{matrix.python-version}} + - name: Run Pytest and get code coverage for Codecov run: > - pytest --cov scripts scripts/ -vv -rA + pytest + --cov=scripts --cov=ocaml/xcp-rrdd + scripts/ ocaml/xcp-rrdd -vv -rA --junitxml=.git/pytest${{matrix.python-version}}.xml --cov-report term-missing --cov-report xml:.git/coverage${{matrix.python-version}}.xml diff --git a/ocaml/xcp-rrdd/scripts/rrdd/rrdd.py b/ocaml/xcp-rrdd/scripts/rrdd/rrdd.py index 6cb7540dcdf..a5dadf326c8 100644 --- a/ocaml/xcp-rrdd/scripts/rrdd/rrdd.py +++ b/ocaml/xcp-rrdd/scripts/rrdd/rrdd.py @@ -100,7 +100,7 @@ class Proxy(xmlrpc.client.ServerProxy): def __init__(self, uri, transport=None, encoding=None, verbose=0, allow_none=1): xmlrpc.client.ServerProxy.__init__(self, uri, transport, encoding, - verbose, allow_none) + bool(verbose), bool(allow_none)) self.transport = transport def request(self, methodname, params): @@ -248,11 +248,11 @@ def set_datasource(self, name, value, description="", units="", def get_header(self): """Get the 'static' first line of the expected output format.""" - return self.header + return self.header # pytype: disable=attribute-error def get_path(self): """Get the path of the file in which to write the results to.""" - return self.path + return self.path # pytype: disable=attribute-error def register(self): """Register plugin if not already registered, and return next_reading.""" diff --git a/ocaml/xcp-rrdd/scripts/rrdd/test_api_wait_until_next_reading.py b/ocaml/xcp-rrdd/scripts/rrdd/test_api_wait_until_next_reading.py new file mode 100644 index 00000000000..5ca9b897fad --- /dev/null +++ b/ocaml/xcp-rrdd/scripts/rrdd/test_api_wait_until_next_reading.py @@ -0,0 +1,79 @@ +# Test: pytest -v -s ocaml/xcp-rrdd/scripts/rrdd/test_api_wait_until_next_reading.py +"""Parametrized test exercising all conditions in rrdd.API.wait_until_next_reading()""" +import socket +from warnings import catch_warnings as import_without_warnings, simplefilter + +# Dependencies: +# pip install pytest-mock +import pytest + +# Handle DeprecationWarning from importing imp (it was removed with Python 3.12) +with import_without_warnings(): + simplefilter(action="ignore", category=DeprecationWarning) + import rrdd + + +# pylint:disable=no-member,redefined-outer-name # pytest fixture, see below + + +@pytest.fixture +def api(mocker): + """Pytest fixture for creating a rrdd.API() instance""" + instance = rrdd.API("plugin_id") + instance.deregister = mocker.Mock() + return instance + + +# pylint:disable=too-many-arguments # pytest parametrized test, see below +@pytest.mark.parametrize( + "neg_shift, interval, reading, sleep", + [ + # Happy path tests with various realistic test values + (None, 5, (6,), 5), # Test the default value of neg_shift + (1, 5, (6,), 5), # to call in the same sleep as neg_shift=1 + (2.25, 5, (6,), 3.75), # Test neg_shift as float to get sleep as float + (0.5, 30, (30.5,), 30), # Also as a fraction of a second + (2, 120, (122,), 120), # Test large interval and reading + # Edge cases + (11, 5, (1,), 0), # large neg_shift results in no sleep + (1, 10, (1,), 0), # neg_shift equals reading from xcp-rrdd + (1, 9, (10,), 9), # wait_time is exactly one cycle + (1, 10, (9,), 8), # wait_time is negative, should wrap around + # Error case + (1, 7, (socket.error, 6), 5), # first register raises socket.error + ], +) +def test_params(api, mocker, neg_shift, interval, reading, sleep, capsys): + """Test that wait_until_reading_from_xcp_rrd() works with various test values""" + # Arrange + api.frequency_in_seconds = interval + api.lazy_complete_init = mocker.Mock() + api.register = mocker.Mock(side_effect=reading) + api.deregister = mocker.Mock() + + # Act + mock_sleep = mocker.patch("time.sleep") + if neg_shift is None: + rrdd.API.wait_until_next_reading(api) + else: + rrdd.API.wait_until_next_reading(api, neg_shift) + + # Assert + mock_sleep.assert_called_with(sleep) + + with capsys.disabled(): + stderr = capsys.readouterr().err + stdout = capsys.readouterr().out + if reading[0] is socket.error: + assert stderr == "Failed to contact xcp-rrdd. Sleeping for 5 seconds ..\n" + else: + assert stderr == "" + assert stdout == "" + + +def test_api_getter_functions(api): + """Test that the API getter functions work (and cover the code)""" + api.header = "header" + api.path = "path" + assert api.get_header() == "header" + assert api.get_path() == "path"