Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small refactor of Builders/Builder.py #273

34 changes: 34 additions & 0 deletions .github/workflows/python_package.yml
Original file line number Diff line number Diff line change
@@ -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"
jurgenwigg marked this conversation as resolved.
Show resolved Hide resolved
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
6 changes: 3 additions & 3 deletions HaikuPorter/BuildMaster.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions HaikuPorter/Builders/Builder.py
Original file line number Diff line number Diff line change
@@ -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'
Expand Down
4 changes: 2 additions & 2 deletions HaikuPorter/Builders/LocalBuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import stat
import time

from .Builder import _BuilderState
from .Builder import BuilderState


class LocalBuilder(object):
Expand All @@ -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')
Expand Down
22 changes: 11 additions & 11 deletions HaikuPorter/Builders/RemoteBuilderSSH.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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()
Expand All @@ -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):
Expand Down Expand Up @@ -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:
Expand All @@ -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))
Expand Down
Empty file added tests/Builders/__init__.py
Empty file.
31 changes: 31 additions & 0 deletions tests/Builders/test_builder.py
Original file line number Diff line number Diff line change
@@ -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
Empty file added tests/__init__.py
Empty file.
3 changes: 3 additions & 0 deletions tests/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pytest
pytest-cov
tox
13 changes: 1 addition & 12 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -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