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

@@ -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

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)

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'

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')

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))

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

tests/requirements.txt

@@ -0,0 +1,3 @@
+pytest
+pytest-cov
+tox

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