From ea103482e3eb19becd2219a5a008f00f9a4083fd Mon Sep 17 00:00:00 2001 From: "Christian Y. Brenninkmeijer" Date: Wed, 3 Jan 2024 09:39:21 +0000 Subject: [PATCH 01/12] fail on ANY pylint message --- .github/workflows/python_actions.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python_actions.yml b/.github/workflows/python_actions.yml index 9afdd8a..f5a1374 100644 --- a/.github/workflows/python_actions.yml +++ b/.github/workflows/python_actions.yml @@ -65,8 +65,9 @@ jobs: - name: Lint with pylint uses: ./support/actions/pylint with: - package: spinnaker_testbase - exitcheck: 39 + package: ${{ env.BASE_PKG }} + exitcheck: 31 # Action fails on any message + language: en_GB - name: Run rat copyright enforcement if: matrix.python-version == 3.8 From 80e7df9c24a1cd4c94397a5a54377657b2280112 Mon Sep 17 00:00:00 2001 From: "Christian Y. Brenninkmeijer" Date: Wed, 3 Jan 2024 10:11:28 +0000 Subject: [PATCH 02/12] 'Exception' is not a proper value for the 'overgeneral-exceptions' option --- .pylintrc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pylintrc b/.pylintrc index 6614a1a..4157151 100644 --- a/.pylintrc +++ b/.pylintrc @@ -467,4 +467,4 @@ known-third-party=enchant # Exceptions that will emit a warning when being caught. Defaults to # "Exception" -overgeneral-exceptions=Exception +overgeneral-exceptions=builtin.Exception From 0aa2d8ce9323a26507408aa8398fe54318a81f4a Mon Sep 17 00:00:00 2001 From: "Christian Y. Brenninkmeijer" Date: Wed, 3 Jan 2024 10:14:49 +0000 Subject: [PATCH 03/12] fix package to pylint --- .github/workflows/python_actions.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python_actions.yml b/.github/workflows/python_actions.yml index f5a1374..5528b5d 100644 --- a/.github/workflows/python_actions.yml +++ b/.github/workflows/python_actions.yml @@ -65,7 +65,7 @@ jobs: - name: Lint with pylint uses: ./support/actions/pylint with: - package: ${{ env.BASE_PKG }} + package: spinnaker_testbase exitcheck: 31 # Action fails on any message language: en_GB From 6941e03f644f489f478e99732d190246d0106b40 Mon Sep 17 00:00:00 2001 From: "Christian Y. Brenninkmeijer" Date: Wed, 3 Jan 2024 10:24:19 +0000 Subject: [PATCH 04/12] module docstring --- README.md | 6 +++--- spinnaker_testbase/__init__.py | 7 +++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 60669e7..7e4c189 100644 --- a/README.md +++ b/README.md @@ -1,12 +1,12 @@ [![Python Actions](https://github.com/SpiNNakerManchester/TestBase/actions/workflows/python_actions.yml/badge.svg?branch=main)](https://github.com/SpiNNakerManchester/TestBase/actions/workflows/python_actions.yml) [![Coverage Status](https://coveralls.io/repos/github/SpiNNakerManchester/TestBase/badge.svg)](https://coveralls.io/github/SpiNNakerManchester/TestBase) -This Repository hold classes and script used for integration tests +This Repository hold classes and script used for unit and integration tests -There is need to use this repository unless you want to run some or all integration tests locally +There is need to use this repository unless you want to run some or all tests locally Documentation ------------- [TestBase documentation](https://spinnakertestbase.readthedocs.io/) -[Combined PyNN8 python documentation (Excluding TestBase)](http://spinnakermanchester.readthedocs.io) +[Combined python documentation (Excluding TestBase)](http://spinnakermanchester.readthedocs.io) diff --git a/spinnaker_testbase/__init__.py b/spinnaker_testbase/__init__.py index 2d8b5aa..148bd8c 100644 --- a/spinnaker_testbase/__init__.py +++ b/spinnaker_testbase/__init__.py @@ -12,6 +12,13 @@ # See the License for the specific language governing permissions and # limitations under the License. +""" +This Repository hold classes and script used for unit and integration tests + +There is need to use this repository unless you want to run some or all tests +locally +""" + from .base_test_case import BaseTestCase from .root_script_builder import RootScriptBuilder from .script_checker import ScriptChecker From 4266318c1e0270f1d9677b4f260fca19560c65fb Mon Sep 17 00:00:00 2001 From: "Christian Y. Brenninkmeijer" Date: Wed, 3 Jan 2024 11:16:38 +0000 Subject: [PATCH 05/12] module docstring --- spinnaker_testbase/base_test_case.py | 3 +++ spinnaker_testbase/root_script_builder.py | 3 +++ spinnaker_testbase/root_test_case.py | 4 ++++ spinnaker_testbase/script_checker.py | 3 +++ spinnaker_testbase/test_no_job_destroy.py | 4 ++++ 5 files changed, 17 insertions(+) diff --git a/spinnaker_testbase/base_test_case.py b/spinnaker_testbase/base_test_case.py index 2a9cac6..b4e9c5f 100644 --- a/spinnaker_testbase/base_test_case.py +++ b/spinnaker_testbase/base_test_case.py @@ -23,6 +23,9 @@ class BaseTestCase(RootTestCase): + """ + This extends unittest.TestCase to offer extra functions as needed. + """ def setUp(self): self._setUp(sys.modules[self.__module__].__file__) diff --git a/spinnaker_testbase/root_script_builder.py b/spinnaker_testbase/root_script_builder.py index 7c60dee..5f84247 100644 --- a/spinnaker_testbase/root_script_builder.py +++ b/spinnaker_testbase/root_script_builder.py @@ -24,6 +24,9 @@ class RootScriptBuilder(object): + """ + Looks for example scripts that can be made into Integeration tests. + """ def add_script(self, test_file, name, local_path, skip_imports): """ diff --git a/spinnaker_testbase/root_test_case.py b/spinnaker_testbase/root_test_case.py index c5bde22..756b6d8 100644 --- a/spinnaker_testbase/root_test_case.py +++ b/spinnaker_testbase/root_test_case.py @@ -29,6 +29,10 @@ class RootTestCase(unittest.TestCase): + """ + This holds the code shared by the all test and script checkers + + """ def _setUp(self, script): # Remove random effect for testing diff --git a/spinnaker_testbase/script_checker.py b/spinnaker_testbase/script_checker.py index 625f435..52b3378 100644 --- a/spinnaker_testbase/script_checker.py +++ b/spinnaker_testbase/script_checker.py @@ -32,6 +32,9 @@ def mockshow(): class ScriptChecker(RootTestCase): + """ + Will run a script. Typically as part of Integration Tests. + """ def script_path(self, script): class_file = sys.modules[self.__module__].__file__ diff --git a/spinnaker_testbase/test_no_job_destroy.py b/spinnaker_testbase/test_no_job_destroy.py index e426ccb..f8ec21f 100644 --- a/spinnaker_testbase/test_no_job_destroy.py +++ b/spinnaker_testbase/test_no_job_destroy.py @@ -18,6 +18,10 @@ class TestNoJobDestory(BaseTestCase): + """ + Used by Jenkins to check if a job was destroyed or something else wrote + to the error file. + """ def test_no_destory_file(self): if os.path.exists(self.error_file()): From d5f97249509b66621b19d701a9bf6f7d98a36fac Mon Sep 17 00:00:00 2001 From: "Christian Y. Brenninkmeijer" Date: Wed, 3 Jan 2024 11:29:39 +0000 Subject: [PATCH 06/12] add docs --- spinnaker_testbase/test_no_job_destroy.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spinnaker_testbase/test_no_job_destroy.py b/spinnaker_testbase/test_no_job_destroy.py index f8ec21f..a581ac6 100644 --- a/spinnaker_testbase/test_no_job_destroy.py +++ b/spinnaker_testbase/test_no_job_destroy.py @@ -19,11 +19,15 @@ class TestNoJobDestory(BaseTestCase): """ - Used by Jenkins to check if a job was destroyed or something else wrote - to the error file. + Used by Jenkins to check if a job was destroyed. """ def test_no_destory_file(self): + """ + Checks for the error file and prints it out if found + + :raise AssertionError: if the error file exists + """ if os.path.exists(self.error_file()): with open(self.error_file(), encoding="utf-8") as error_file: error_text = error_file.read() From b28d9770bb2f5672d4535c39bdd8ee6f1cfe00fd Mon Sep 17 00:00:00 2001 From: "Christian Y. Brenninkmeijer" Date: Wed, 3 Jan 2024 11:48:06 +0000 Subject: [PATCH 07/12] test --- spinnaker_testbase/ping.py | 76 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 spinnaker_testbase/ping.py diff --git a/spinnaker_testbase/ping.py b/spinnaker_testbase/ping.py new file mode 100644 index 0000000..cdb4d62 --- /dev/null +++ b/spinnaker_testbase/ping.py @@ -0,0 +1,76 @@ +# Copyright (c) 2018 The University of Manchester +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import platform +import subprocess +import time +from typing import Set + + +class Ping(object): + """ + Platform-independent ping support. + """ + + #: The unreachable host cache. + unreachable: Set[str] = set() + + @staticmethod + def ping(ip_address): + """ + Send a ping (ICMP ECHO request) to the given host. + SpiNNaker boards support ICMP ECHO when booted. + + :param str ip_address: + The IP address to ping. Hostnames can be used, but are not + recommended. + :return: + return code of subprocess; 0 for success, anything else for failure + :rtype: int + """ + if platform.platform().lower().startswith("windows"): + cmd = "ping -n 1 -w 1 " + else: + cmd = "ping -c 1 -W 1 " + process = subprocess.Popen( + cmd + ip_address, shell=True, stdout=subprocess.PIPE) + time.sleep(1.2) + process.stdout.close() + process.wait() + return process.returncode + + @staticmethod + def host_is_reachable(ip_address): + """ + Test if a host is unreachable via ICMP ECHO. + + .. note:: + This information may be cached in various ways. Transient failures + are not necessarily detected or recovered from. + + :param str ip_address: + The IP address to ping. Hostnames can be used, but are not + recommended. + :rtype: bool + """ + if ip_address in Ping.unreachable: + return False + tries = 0 + while (True): + if Ping.ping(ip_address) == 0: + return True + tries += 1 + if tries > 10: + Ping.unreachable.add(ip_address) + return False From 064c411d7b4188f10eece8e9b216c7025e818289 Mon Sep 17 00:00:00 2001 From: "Christian Y. Brenninkmeijer" Date: Thu, 4 Jan 2024 15:40:19 +0000 Subject: [PATCH 08/12] updated whi9ch checks pylint should run --- .pylintrc | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/.pylintrc b/.pylintrc index 4157151..32418d4 100644 --- a/.pylintrc +++ b/.pylintrc @@ -68,13 +68,20 @@ confidence= # --enable=similarities". If you want to run only the classes checker, but have # no Warning level messages displayed, use"--disable=all --enable=classes # --disable=W" -disable= +disable=consider-using-dict-items, missing-module-docstring, unsubscriptable-object + +# consider-using-dict-items "for x in foo:" is fine! + +# missing-module-docstring expects a comment at import level which we don't do + +# False positives for unsubscriptable-object. Mypy better at this class of issue +# See https://github.com/pylint-dev/pylint/issues/1498 # Enable the message, report, category or checker with the given id(s). You can # either give multiple identifier separated by comma (,) or put this option # multiple time (only on the command line, not in the configuration file where # it should appear only once). See also the "--disable" option for examples. -enable=c-extension-no-member +enable= [REPORTS] @@ -265,7 +272,6 @@ notes=FIXME, XXX - [SIMILARITIES] # Ignore comments when computing similarities. @@ -291,7 +297,7 @@ max-spelling-suggestions=4 spelling-dict= # List of comma separated words that should not be checked. -spelling-ignore-words= +spelling-ignore-words=ReservedAssignment,noqa,pragma # A path to a file that contains private dictionary; one word per line. spelling-private-dict-file= @@ -379,8 +385,9 @@ init-import=no # List of method names used to declare (i.e. assign) instance attributes. defining-attr-methods=__init__, __new__, - _new_run_clear, - _machine_clear, + __call__, + _hard_reset, + setUp # List of member names, which should be excluded from the protected access # warning. From 7a1e595e0fe35626c94f7cf6212820ec7e2cf11a Mon Sep 17 00:00:00 2001 From: "Christian Y. Brenninkmeijer" Date: Thu, 4 Jan 2024 16:44:29 +0000 Subject: [PATCH 09/12] pylint fixes or disable --- .pylint_dict.txt | 30 +++++++++++++++++++++++ spinnaker_testbase/base_test_case.py | 28 ++++++++++++++++----- spinnaker_testbase/ping.py | 2 +- spinnaker_testbase/root_script_builder.py | 2 +- spinnaker_testbase/root_test_case.py | 21 ++++++++++++---- spinnaker_testbase/script_checker.py | 7 +++--- 6 files changed, 74 insertions(+), 16 deletions(-) create mode 100644 .pylint_dict.txt diff --git a/.pylint_dict.txt b/.pylint_dict.txt new file mode 100644 index 0000000..d346f98 --- /dev/null +++ b/.pylint_dict.txt @@ -0,0 +1,30 @@ +# Copyright (c) 2023 The University of Manchester +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Our abbreviations/names + +# Python types +TextIOBase + +# Python packages +pydevd +unittest + +# Our "special" words +dirs +xyz + +# Python bits + +# Misc diff --git a/spinnaker_testbase/base_test_case.py b/spinnaker_testbase/base_test_case.py index b4e9c5f..376c798 100644 --- a/spinnaker_testbase/base_test_case.py +++ b/spinnaker_testbase/base_test_case.py @@ -15,6 +15,7 @@ import os import random import sys +from typing import List from spinn_front_end_common.data import FecDataView from .root_test_case import RootTestCase @@ -28,7 +29,7 @@ class BaseTestCase(RootTestCase): """ def setUp(self): - self._setUp(sys.modules[self.__module__].__file__) + self._setup(sys.modules[self.__module__].__file__) def assert_logs_messages( self, log_records, sub_message, log_level='ERROR', count=1, @@ -54,14 +55,29 @@ def assert_logs_messages( f'"{sub_message}" not found in any {log_level} logs ' f'{count} times, was found {seen} times') - def get_provenance_files(self): + def get_provenance_files(self) -> List[str]: + """ + Gets a list of the Provenance files + + :rtype: list(str) + """ provenance_file_path = FecDataView().get_provenance_dir_path() return os.listdir(provenance_file_path) - def get_system_iobuf_files(self): - system_iobuf_file_path = (FecDataView.get_system_provenance_dir_path()) + def get_system_iobuf_files(self) -> List[str]: + """ + Get a list of the system iobug files. + + :rtype: list(str) + """ + system_iobuf_file_path = FecDataView.get_system_provenance_dir_path() return os.listdir(system_iobuf_file_path) - def get_app_iobuf_files(self): - app_iobuf_file_path = (FecDataView.get_app_provenance_dir_path()) + def get_app_iobuf_files(self) -> List[str]: + """ + Get a list of the application iobug files. + + :rtype: list(str) + """ + app_iobuf_file_path = FecDataView.get_app_provenance_dir_path() return os.listdir(app_iobuf_file_path) diff --git a/spinnaker_testbase/ping.py b/spinnaker_testbase/ping.py index cdb4d62..0190908 100644 --- a/spinnaker_testbase/ping.py +++ b/spinnaker_testbase/ping.py @@ -67,7 +67,7 @@ def host_is_reachable(ip_address): if ip_address in Ping.unreachable: return False tries = 0 - while (True): + while True: if Ping.ping(ip_address) == 0: return True tries += 1 diff --git a/spinnaker_testbase/root_script_builder.py b/spinnaker_testbase/root_script_builder.py index 5f84247..4b35056 100644 --- a/spinnaker_testbase/root_script_builder.py +++ b/spinnaker_testbase/root_script_builder.py @@ -25,7 +25,7 @@ class RootScriptBuilder(object): """ - Looks for example scripts that can be made into Integeration tests. + Looks for example scripts that can be made into integration tests. """ def add_script(self, test_file, name, local_path, skip_imports): diff --git a/spinnaker_testbase/root_test_case.py b/spinnaker_testbase/root_test_case.py index 756b6d8..823ad90 100644 --- a/spinnaker_testbase/root_test_case.py +++ b/spinnaker_testbase/root_test_case.py @@ -23,9 +23,9 @@ from spinn_front_end_common.data import FecDataView if os.environ.get('CONTINUOUS_INTEGRATION', 'false').lower() == 'true': - max_tries = 3 + MAX_TRIES = 3 else: - max_tries = 1 + MAX_TRIES = 1 class RootTestCase(unittest.TestCase): @@ -34,7 +34,7 @@ class RootTestCase(unittest.TestCase): """ - def _setUp(self, script): + def _setup(self, script): # Remove random effect for testing # Set test_seed to None to allow random # pylint: disable=attribute-defined-outside-init @@ -66,6 +66,17 @@ def error_file(self): return os.path.join(test_dir, "ErrorFile.txt") def report(self, message, file_name): + """ + Writes some text to the specified file + + The file will be written in the env GLOBAL_REPORTS directory. + + If no GLOBAL_REPORTS is defined the timestamp directory + holding the run data is used. + + :param str message: + :param str file_name: local file name. + """ if not message.endswith("\n"): message += "\n" global_reports = os.environ.get("GLOBAL_REPORTS", None) @@ -73,7 +84,7 @@ def report(self, message, file_name): try: global_reports = FecDataView.get_timestamp_dir_path() except NotSetupException: - # This may happen if you are running none script fiels locally + # This may happen if you are running none script files locally return if not os.path.exists(global_reports): @@ -116,7 +127,7 @@ def runsafe(self, method, retry_delay=3.0, skip_exceptions=None): error_file.write(str(ex)) error_file.write("\n") retries += 1 - if retries >= max_tries: + if retries >= MAX_TRIES: raise ex except (PacmanValueError, PacmanPartitionException) as ex: # skip out if on a spin three diff --git a/spinnaker_testbase/script_checker.py b/spinnaker_testbase/script_checker.py index 52b3378..1c606a4 100644 --- a/spinnaker_testbase/script_checker.py +++ b/spinnaker_testbase/script_checker.py @@ -15,12 +15,13 @@ import os import sys import time +from unittest import SkipTest import matplotlib import matplotlib.pyplot as pyplot -from unittest import SkipTest from .root_test_case import RootTestCase matplotlib.use('Agg') +# pylint: disable=invalid-name script_checker_shown = False @@ -56,8 +57,8 @@ def check_script(self, script, broken_msg=None, skip_exceptions=None): global script_checker_shown script_path = self.script_path(script) - self._setUp(script_path) - + self._setup(script_path) + # pylint: disable=import-outside-toplevel plotting = "import matplotlib.pyplot" in ( open(script_path, encoding="utf-8").read()) if plotting: From 1ac8b826bf3ddf8e1f3cae67456da05f3823af85 Mon Sep 17 00:00:00 2001 From: "Christian Y. Brenninkmeijer" Date: Fri, 5 Jan 2024 06:32:09 +0000 Subject: [PATCH 10/12] pylint fixes --- .pylint_dict.txt | 2 ++ spinnaker_testbase/base_test_case.py | 4 ++-- spinnaker_testbase/script_checker.py | 5 +++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/.pylint_dict.txt b/.pylint_dict.txt index d346f98..c7c89cd 100644 --- a/.pylint_dict.txt +++ b/.pylint_dict.txt @@ -23,8 +23,10 @@ unittest # Our "special" words dirs +iobuf xyz # Python bits +env # Misc diff --git a/spinnaker_testbase/base_test_case.py b/spinnaker_testbase/base_test_case.py index 376c798..03087b0 100644 --- a/spinnaker_testbase/base_test_case.py +++ b/spinnaker_testbase/base_test_case.py @@ -66,7 +66,7 @@ def get_provenance_files(self) -> List[str]: def get_system_iobuf_files(self) -> List[str]: """ - Get a list of the system iobug files. + Get a list of the system iobuf files. :rtype: list(str) """ @@ -75,7 +75,7 @@ def get_system_iobuf_files(self) -> List[str]: def get_app_iobuf_files(self) -> List[str]: """ - Get a list of the application iobug files. + Get a list of the application iobuf files. :rtype: list(str) """ diff --git a/spinnaker_testbase/script_checker.py b/spinnaker_testbase/script_checker.py index 1c606a4..04823b6 100644 --- a/spinnaker_testbase/script_checker.py +++ b/spinnaker_testbase/script_checker.py @@ -27,6 +27,11 @@ # This is a global function as pydevd calls _needsmain when debugging def mockshow(): + """ + This will replace pyplot.show during script tests + + This avoids the plots from printed but checks the script tried to + """ # pylint: disable=global-statement global script_checker_shown script_checker_shown = True From 5b5c11a370614fc37077d6467cc5430f19366112 Mon Sep 17 00:00:00 2001 From: "Christian Y. Brenninkmeijer" Date: Fri, 5 Jan 2024 07:01:18 +0000 Subject: [PATCH 11/12] accept spelling pyplot --- .pylint_dict.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/.pylint_dict.txt b/.pylint_dict.txt index c7c89cd..9d3a074 100644 --- a/.pylint_dict.txt +++ b/.pylint_dict.txt @@ -19,6 +19,7 @@ TextIOBase # Python packages pydevd +pyplot unittest # Our "special" words From 63536bb5b34dd623d68d990fe1094c03f5034bd1 Mon Sep 17 00:00:00 2001 From: "Christian Y. Brenninkmeijer" Date: Fri, 5 Jan 2024 07:05:02 +0000 Subject: [PATCH 12/12] make method private --- spinnaker_testbase/script_checker.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spinnaker_testbase/script_checker.py b/spinnaker_testbase/script_checker.py index 04823b6..22c700c 100644 --- a/spinnaker_testbase/script_checker.py +++ b/spinnaker_testbase/script_checker.py @@ -42,7 +42,7 @@ class ScriptChecker(RootTestCase): Will run a script. Typically as part of Integration Tests. """ - def script_path(self, script): + def _script_path(self, script): class_file = sys.modules[self.__module__].__file__ integration_tests_directory = os.path.dirname(class_file) root_dir = os.path.dirname(integration_tests_directory) @@ -61,7 +61,7 @@ def check_script(self, script, broken_msg=None, skip_exceptions=None): # pylint: disable=global-statement global script_checker_shown - script_path = self.script_path(script) + script_path = self._script_path(script) self._setup(script_path) # pylint: disable=import-outside-toplevel plotting = "import matplotlib.pyplot" in (