From 007d8684c90c090a643252267492e2399bf41977 Mon Sep 17 00:00:00 2001 From: jurgenwigg <53076001+jurgenwigg@users.noreply.github.com> Date: Mon, 9 Oct 2023 20:11:53 +0200 Subject: [PATCH] Small refactor of Builders/Builder.py (#273) * refactor: changed Builder.py * refactor: changed name from _BuilderState to BuilderState * refactor: added unit tests for the BuilderStates module * feat: added tox-based CI * feat: removed the newest python version * feat: removed unnecessary entries * feat: simplified workflow * fix: fixing executing tests in Actions * fix: changed allow_externals from poetry to pytest * fix: add pytest-cov as a required package for testing * fix: removed macos and windows testing * refactor: removed commented line --- .github/workflows/python_package.yml | 34 ++++++++++++++++++++++++ HaikuPorter/BuildMaster.py | 6 ++--- HaikuPorter/Builders/Builder.py | 5 ++-- HaikuPorter/Builders/LocalBuilder.py | 4 +-- HaikuPorter/Builders/RemoteBuilderSSH.py | 22 +++++++-------- tests/Builders/__init__.py | 0 tests/Builders/test_builder.py | 31 +++++++++++++++++++++ tests/__init__.py | 0 tests/requirements.txt | 3 +++ tox.ini | 13 +-------- 10 files changed, 87 insertions(+), 31 deletions(-) create mode 100644 .github/workflows/python_package.yml create mode 100644 tests/Builders/__init__.py create mode 100644 tests/Builders/test_builder.py create mode 100644 tests/__init__.py create mode 100644 tests/requirements.txt diff --git a/.github/workflows/python_package.yml b/.github/workflows/python_package.yml new file mode 100644 index 00000000..5ae6869a --- /dev/null +++ b/.github/workflows/python_package.yml @@ -0,0 +1,34 @@ +name: check +on: + push: + pull_request: + +concurrency: + group: check-${{ github.ref }} + cancel-in-progress: true + +jobs: + test: + name: test with ${{ matrix.py }} on ${{ matrix.os }} + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + py: + - "3.11" + - "3.10" + - "3.9" + os: + - ubuntu-latest + steps: + - uses: actions/checkout@v3 + with: + fetch-depth: 0 + - name: Setup python for test ${{ matrix.py }} + uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.py }} + - name: Install tox + run: python -m pip install -r tests/requirements.txt + - name: Run test suite + run: tox diff --git a/HaikuPorter/BuildMaster.py b/HaikuPorter/BuildMaster.py index 242f0695..edeb7c8a 100644 --- a/HaikuPorter/BuildMaster.py +++ b/HaikuPorter/BuildMaster.py @@ -14,7 +14,7 @@ import threading import time -from .Builders.Builder import _BuilderState +from .Builders.Builder import BuilderState from .Builders.LocalBuilder import LocalBuilder from .Builders.RemoteBuilderSSH import RemoteBuilderSSH from .Configuration import Configuration @@ -514,11 +514,11 @@ def _buildThread(self, builder, scheduledBuild, buildNumber): self.completeBuilds if buildSuccess else self.failedBuilds) with self.builderCondition: - if builder.state == _BuilderState.LOST: + if builder.state == BuilderState.LOST: self.logger.error('builder ' + builder.name + ' lost') self.activeBuilders.remove(builder) self.lostBuilders.append(builder) - elif builder.state == _BuilderState.RECONNECT: + elif builder.state == BuilderState.RECONNECT: self.logger.error( 'builder ' + builder.name + ' is reconnecting') self.activeBuilders.remove(builder) diff --git a/HaikuPorter/Builders/Builder.py b/HaikuPorter/Builders/Builder.py index edd737cb..c54fd296 100644 --- a/HaikuPorter/Builders/Builder.py +++ b/HaikuPorter/Builders/Builder.py @@ -1,10 +1,9 @@ -# -*- coding: utf-8 -*- -# # Copyright 2015 Michael Lotz # Copyright 2016 Jerome Duval # Distributed under the terms of the MIT License. -class _BuilderState(object): +class BuilderState: + """Provides mapping of builder state to string.""" AVAILABLE = 'Available' LOST = 'Lost' NOT_AVAILABLE = 'Not Available' diff --git a/HaikuPorter/Builders/LocalBuilder.py b/HaikuPorter/Builders/LocalBuilder.py index 871a371a..8985816e 100644 --- a/HaikuPorter/Builders/LocalBuilder.py +++ b/HaikuPorter/Builders/LocalBuilder.py @@ -10,7 +10,7 @@ import stat import time -from .Builder import _BuilderState +from .Builder import BuilderState class LocalBuilder(object): @@ -20,7 +20,7 @@ def __init__(self, name, packagesPath, outputBaseDir, options): self.buildCount = 0 self.failedBuilds = 0 self.packagesPath = packagesPath - self.state = _BuilderState.AVAILABLE + self.state = BuilderState.AVAILABLE self.currentBuild = None self.buildOutputDir = os.path.join(outputBaseDir, 'builds') diff --git a/HaikuPorter/Builders/RemoteBuilderSSH.py b/HaikuPorter/Builders/RemoteBuilderSSH.py index caae26ce..e8637178 100644 --- a/HaikuPorter/Builders/RemoteBuilderSSH.py +++ b/HaikuPorter/Builders/RemoteBuilderSSH.py @@ -15,7 +15,7 @@ # These usages kinda need refactored from ..ConfigParser import ConfigParser from ..Configuration import Configuration -from .Builder import _BuilderState +from .Builder import BuilderState try: import paramiko @@ -43,7 +43,7 @@ def __init__(self, configFilePath, packagesPath, outputBaseDir, if not os.path.isdir(self.buildOutputDir): os.makedirs(self.buildOutputDir) - self.state = _BuilderState.NOT_AVAILABLE + self.state = BuilderState.NOT_AVAILABLE self.connectionErrors = 0 self.maxConnectionErrors = 100 @@ -141,13 +141,13 @@ def _connect(self): + str(exception)) self.connectionErrors += 1 - self.state = _BuilderState.RECONNECT + self.state = BuilderState.RECONNECT if self.connectionErrors >= self.maxConnectionErrors: self.logger.error('giving up on builder after ' + str(self.connectionErrors) + ' consecutive connection errors') - self.state = _BuilderState.LOST + self.state = BuilderState.LOST raise # avoid DoSing the remote host, increasing delay as retries increase. @@ -240,9 +240,9 @@ def _removeObsoletePackages(self): self.availablePackages.remove(entry) def _setupForBuilding(self): - if self.state == _BuilderState.AVAILABLE: + if self.state == BuilderState.AVAILABLE: return True - if self.state == _BuilderState.LOST: + if self.state == BuilderState.LOST: return False self._connect() @@ -253,7 +253,7 @@ def _setupForBuilding(self): self._getAvailablePackages() self._removeObsoletePackages() - self.state = _BuilderState.AVAILABLE + self.state = BuilderState.AVAILABLE return True def setBuild(self, scheduledBuild, buildNumber): @@ -318,7 +318,7 @@ def runBuild(self): self.buildLogger.info('command exit status: ' + str(exitStatus)) if exitStatus < 0 and not channel.get_transport().is_active(): - self.state = _BuilderState.NOT_AVAILABLE + self.state = BuilderState.NOT_AVAILABLE raise Exception('builder disconnected') if exitStatus != 0: @@ -344,12 +344,12 @@ def runBuild(self): except socket.error as exception: self.buildLogger.error('connection failed: ' + str(exception)) - if self.state == _BuilderState.AVAILABLE: - self.state = _BuilderState.RECONNECT + if self.state == BuilderState.AVAILABLE: + self.state = BuilderState.RECONNECT except (IOError, paramiko.ssh_exception.SSHException) as exception: self.buildLogger.error('builder failed: ' + str(exception)) - self.state = _BuilderState.LOST + self.state = BuilderState.LOST except Exception as exception: self.buildLogger.info('build failed: ' + str(exception)) diff --git a/tests/Builders/__init__.py b/tests/Builders/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/Builders/test_builder.py b/tests/Builders/test_builder.py new file mode 100644 index 00000000..199165ad --- /dev/null +++ b/tests/Builders/test_builder.py @@ -0,0 +1,31 @@ +# Copyright 2023 @jurgenwigg +# Distributed under the terms of the MIT License. +"""Unit tests for Builder.py module""" +from HaikuPorter.Builders.Builder import BuilderState +from pytest import mark + + +def test_number_of_possible_states(): + """Tests number of possible states. + + We assume that only 4 possible states are possible. + """ + expected_value = 4 + actual_value = len( + [item for item in BuilderState.__dict__.keys() if not item.startswith("__")] + ) + assert expected_value == actual_value + + +@mark.parametrize( + "state, expected_result", + [ + ["AVAILABLE", "Available"], + ["LOST", "Lost"], + ["NOT_AVAILABLE", "Not Available"], + ["RECONNECT", "Reconnecting"], + ], +) +def test_is_state_present(state, expected_result): + """Tests if given state returns expected state string.""" + assert BuilderState.__dict__[state] == expected_result diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/requirements.txt b/tests/requirements.txt new file mode 100644 index 00000000..8b0af441 --- /dev/null +++ b/tests/requirements.txt @@ -0,0 +1,3 @@ +pytest +pytest-cov +tox \ No newline at end of file diff --git a/tox.ini b/tox.ini index ed3e35ca..003e9421 100644 --- a/tox.ini +++ b/tox.ini @@ -1,18 +1,7 @@ [tox] skipsdist = true -envlist = py38, py39, py310, py311 - -[gh-actions] -python = - 3.8: py38 - 3.9: py39 - 3.10: py310 - 3.11: py311 [testenv] -passenv = PYTHON_VERSION -allowlist_externals = poetry +allowlist_externals = pytest commands = - poetry install -v pytest --doctest-modules tests --cov --cov-config=pyproject.toml --cov-report=xml - mypy