Skip to content

Commit

Permalink
Test fixes (#943)
Browse files Browse the repository at this point in the history
* SI Units also in tests for consistency

* macOS cannot run network provider tests

* Reducing expectation for VM values in macOS

* Making GlobalConfig immutable; Changing runner to handle conf differently

* Skipping sleeps when testing

* Config override must be a full path

* RElaxing network condition a little

* Reset of the config was not happening correctly
  • Loading branch information
ArneTR authored Oct 10, 2024
1 parent 2cb2c69 commit 98fce53
Show file tree
Hide file tree
Showing 19 changed files with 222 additions and 93 deletions.
27 changes: 14 additions & 13 deletions cron/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import subprocess
import json
import argparse
from pathlib import Path

from lib.job.base import Job
from lib.global_config import GlobalConfig
Expand Down Expand Up @@ -75,7 +74,7 @@ def do_cleanup(cur_temp, cooldown_time_after_job):
try:
parser = argparse.ArgumentParser()
parser.add_argument('--testing', action='store_true', help='End after processing one run for testing')
parser.add_argument('--config-override', type=str, help='Override the configuration file with the passed in yml file. Must be located in the same directory as the regular configuration file. Pass in only the name.')
parser.add_argument('--config-override', type=str, help='Override the configuration file with the passed in yml file. Supply full path.')

args = parser.parse_args()

Expand All @@ -84,11 +83,7 @@ def do_cleanup(cur_temp, cooldown_time_after_job):
parser.print_help()
error_helpers.log_error('Config override file must be a yml file')
sys.exit(1)
if not Path(f"{CURRENT_DIR}/../{args.config_override}").is_file():
parser.print_help()
error_helpers.log_error(f"Could not find config override file on local system. Please double check: {CURRENT_DIR}/../{args.config_override}")
sys.exit(1)
GlobalConfig(config_name=args.config_override) # will create a singleton and subsequent calls will retrieve object with altered default config file
GlobalConfig(config_location=args.config_override) # will create a singleton and subsequent calls will retrieve object with altered default config file

config_main = GlobalConfig().config

Expand All @@ -103,7 +98,8 @@ def do_cleanup(cur_temp, cooldown_time_after_job):
job = Job.get_job('run')
if job and job.check_job_running():
error_helpers.log_error('Job is still running. This is usually an error case! Continuing for now ...', machine=config_main['machine']['description'])
time.sleep(client_main['sleep_time_no_job'])
if not args.testing:
time.sleep(client_main['sleep_time_no_job'])
continue

if not args.testing:
Expand All @@ -120,7 +116,8 @@ def do_cleanup(cur_temp, cooldown_time_after_job):
set_status('cooldown', current_temperature, last_cooldown_time)
cooldown_time += 60
temperature_errors += 1
time.sleep(60)
if not args.testing:
time.sleep(60)
continue

if current_temperature <= (config_main['machine']['base_temperature_value'] - 5):
Expand Down Expand Up @@ -161,7 +158,8 @@ def do_cleanup(cur_temp, cooldown_time_after_job):
# the process will now go to sleep for 'time_between_control_workload_validations''
# This is as long as the next validation is needed and thus it will loop
# endlessly in validation until manually handled, which is what we want.
time.sleep(client_main['time_between_control_workload_validations'])
if not args.testing:
time.sleep(client_main['time_between_control_workload_validations'])
finally:
do_cleanup(current_temperature, last_cooldown_time)

Expand All @@ -174,10 +172,12 @@ def do_cleanup(cur_temp, cooldown_time_after_job):
set_status('job_error', current_temperature, last_cooldown_time, data=str(exc), run_id=job._run_id)
if exc.status == Status.WARN: # Warnings is something like CPU% too high. Here short sleep
error_helpers.log_error('Job processing in cluster failed (client.py)', exception=exc, status=exc.status, run_id=job._run_id, name=job._name, url=job._url, machine=config_main['machine']['description'], sleep_duration=600)
time.sleep(600)
if not args.testing:
time.sleep(600)
else: # Hard fails won't resolve on it's own. We sleep until next cluster validation
error_helpers.log_error('Job processing in cluster failed (client.py)', exception=exc, status=exc.status, run_id=job._run_id, name=job._name, url=job._url, machine=config_main['machine']['description'], sleep_duration=client_main['time_between_control_workload_validations'])
time.sleep(client_main['time_between_control_workload_validations'])
if not args.testing:
time.sleep(client_main['time_between_control_workload_validations'])

except subprocess.CalledProcessError as exc:
set_status('job_error', current_temperature, last_cooldown_time, data=str(exc), run_id=job._run_id)
Expand All @@ -194,7 +194,8 @@ def do_cleanup(cur_temp, cooldown_time_after_job):
set_status('job_no', current_temperature, last_cooldown_time)
if client_main['shutdown_on_job_no'] is True:
subprocess.check_output(['sudo', 'systemctl', 'suspend'])
time.sleep(client_main['sleep_time_no_job'])
if not args.testing:
time.sleep(client_main['sleep_time_no_job'])
if args.testing:
print('Successfully ended testing run of client.py')
break
Expand Down
10 changes: 2 additions & 8 deletions cron/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import os
from datetime import datetime
import argparse
from pathlib import Path

CURRENT_DIR = os.path.dirname(os.path.abspath(__file__))

Expand All @@ -33,7 +32,7 @@
try:
parser = argparse.ArgumentParser()
parser.add_argument('type', help='Select the operation mode.', choices=['email', 'run'])
parser.add_argument('--config-override', type=str, help='Override the configuration file with the passed in yml file. Must be located in the same directory as the regular configuration file. Pass in only the name.')
parser.add_argument('--config-override', type=str, help='Override the configuration file with the passed in yml file. Supply full path.')
parser.add_argument('--skip-system-checks', action='store_true', default=False, help='Skip system checks')
parser.add_argument('--full-docker-prune', action='store_true', default=False, help='Prune all images and build caches on the system')
parser.add_argument('--docker-prune', action='store_true', help='Prune all unassociated build caches, networks volumes and stopped containers on the system')
Expand All @@ -48,12 +47,7 @@
parser.print_help()
error_helpers.log_error('Config override file must be a yml file')
sys.exit(1)
if not Path(f"{CURRENT_DIR}/../{args.config_override}").is_file():
parser.print_help()
error_helpers.log_error(f"Could not find config override file on local system.\
Please double check: {CURRENT_DIR}/../{args.config_override}")
sys.exit(1)
GlobalConfig(config_name=args.config_override)
GlobalConfig(config_location=args.config_override)

job_main = Job.get_job(args.type)
if not job_main:
Expand Down
47 changes: 38 additions & 9 deletions lib/global_config.py
Original file line number Diff line number Diff line change
@@ -1,26 +1,55 @@
import os
import yaml

class FrozenDict(dict):
def __setattr__(self, key, value):
raise TypeError("GlobalConfig is immutable once loaded! (__setattr__)")

def __setitem__(self, key, value):
raise TypeError("GlobalConfig is immutable once loaded! (__setitem__)")

def __delitem__(self, key):
raise TypeError("GlobalConfig is immutable once loaded! (__delitem__)")

def update(self, *args, **kwargs):
raise TypeError("GlobalConfig is immutable once loaded! (update)")

def setdefault(self, *args, **kwargs):
raise TypeError("GlobalConfig is immutable once loaded! (setdefault)")

# Function to recursively freeze nested dictionaries
def freeze_dict(d):
if isinstance(d, dict):
# Convert nested dicts to FrozenDict
return FrozenDict({k: freeze_dict(v) for k, v in d.items()})
if isinstance(d, list):
# Convert lists to tuples (to make them immutable)
return tuple(freeze_dict(item) for item in d)
if not hasattr(d, '__class__') and isinstance(d, object): # hasattr __class__ separates actual defined classes from builtins like str
raise RuntimeError(f"GlobalConfig received object of type {type(d)} in it's config initalization. This is not expected and leads to issues as it cannot be made immutable!")

# Return the object itself if not a dict or list
return d

class GlobalConfig:

# for unknown reasons pylint needs this argument set, although we don't use it. Only in __init__
# pylint: disable=unused-argument
def __new__(cls, config_name='config.yml'):
def __new__(cls, config_location=f"{os.path.dirname(os.path.realpath(__file__))}/../config.yml"):
if not hasattr(cls, 'instance'):
cls.instance = super(GlobalConfig, cls).__new__(cls)
return cls.instance

def __init__(self, config_name='config.yml'):
def __init__(self, config_location=f"{os.path.dirname(os.path.realpath(__file__))}/../config.yml"):
if not hasattr(self, 'config'):
path = os.path.dirname(os.path.realpath(__file__))
with open(f"{path}/../{config_name}", encoding='utf8') as config_file:
self.config = yaml.load(config_file, yaml.FullLoader)
with open(config_location, encoding='utf8') as config_file:
self.config = freeze_dict(yaml.load(config_file, yaml.FullLoader))


## add an override function that will always set the config to a new value
def override_config(self, config_name='config.yml'):
path = os.path.dirname(os.path.realpath(__file__))
with open(f"{path}/../{config_name}", encoding='utf8') as config_file:
self.config = yaml.load(config_file, yaml.FullLoader)
def override_config(self, config_location=f"{os.path.dirname(os.path.realpath(__file__))}/../config.yml"):
with open(config_location, encoding='utf8') as config_file:
self.config = freeze_dict(yaml.load(config_file, yaml.FullLoader))


if __name__ == '__main__':
Expand Down
30 changes: 16 additions & 14 deletions runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -507,18 +507,24 @@ def import_metric_providers(self):
module_path = f"metric_providers.{module_path}"
conf = metric_providers[metric_provider] or {}

if rootless and '.cgroup.' in module_path:
conf['rootless'] = True
print(f"Importing {class_name} from {module_path}")
module = importlib.import_module(module_path)

if self._skip_system_checks:
conf['skip_check'] = True
if rootless and '.cgroup.' in module_path and self._skip_system_checks:
metric_provider_obj = getattr(module, class_name)(**conf, rootless=True, skip_check=True)
print(f"Configuration is {conf}; rootless=true, skip_check=True")
elif rootless and '.cgroup.' in module_path:
metric_provider_obj = getattr(module, class_name)(**conf, rootless=True)
print(f"Configuration is {conf}; rootless=true")
elif self._skip_system_checks:
metric_provider_obj = getattr(module, class_name)(**conf, skip_check=True)
print(f"Configuration is {conf}; skip_check=true")
else:
metric_provider_obj = getattr(module, class_name)(**conf)
print(f"Configuration is {conf}")

print(f"Importing {class_name} from {module_path}")
print(f"Configuration is {conf}")

module = importlib.import_module(module_path)

metric_provider_obj = getattr(module, class_name)(**conf)

self.__metric_providers.append(metric_provider_obj)

Expand Down Expand Up @@ -1641,7 +1647,7 @@ def run(self):
parser.add_argument('--branch', type=str, help='Optionally specify the git branch when targeting a git repository')
parser.add_argument('--name', type=str, help='A name which will be stored to the database to discern this run from others')
parser.add_argument('--filename', type=str, default='usage_scenario.yml', help='An optional alternative filename if you do not want to use "usage_scenario.yml"')
parser.add_argument('--config-override', type=str, help='Override the configuration file with the passed in yml file. Must be located in the same directory as the regular configuration file. Pass in only the name.')
parser.add_argument('--config-override', type=str, help='Override the configuration file with the passed in yml file. Supply full path.')
parser.add_argument('--file-cleanup', action='store_true', help='Delete all temporary files that the runner produced')
parser.add_argument('--debug', action='store_true', help='Activate steppable debug mode')
parser.add_argument('--allow-unsafe', action='store_true', help='Activate unsafe volume bindings, ports and complex environment vars')
Expand Down Expand Up @@ -1699,11 +1705,7 @@ def run(self):
parser.print_help()
error_helpers.log_error('Config override file must be a yml file')
sys.exit(1)
if not Path(f"{CURRENT_DIR}/{args.config_override}").is_file():
parser.print_help()
error_helpers.log_error(f"Could not find config override file on local system. Please double check: {CURRENT_DIR}/{args.config_override}")
sys.exit(1)
GlobalConfig(config_name=args.config_override)
GlobalConfig(config_location=args.config_override)

runner = Runner(name=args.name, uri=args.uri, uri_type=run_type, filename=args.filename,
branch=args.branch, debug_mode=args.debug, allow_unsafe=args.allow_unsafe,
Expand Down
2 changes: 1 addition & 1 deletion tests/api/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from lib.global_config import GlobalConfig
from tests import test_functions as Tests

config = GlobalConfig(config_name='test-config.yml').config
config = GlobalConfig(config_location=f"{os.path.dirname(os.path.realpath(__file__))}/test-config.yml").config
API_URL = config['cluster']['api_url']

from api.main import Software
Expand Down
3 changes: 2 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import pytest
import os

from tests import test_functions as Tests

## VERY IMPORTANT to override the config file here
## otherwise it will automatically connect to non-test DB and delete all your real data
from lib.global_config import GlobalConfig
GlobalConfig().override_config(config_name='test-config.yml')
GlobalConfig().override_config(config_location=f"{os.path.dirname(os.path.realpath(__file__))}/test-config.yml")

def pytest_collection_modifyitems(items):
for item in items:
Expand Down
4 changes: 2 additions & 2 deletions tests/cron/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from lib.job.base import Job
from tests import test_functions as Tests

GlobalConfig().override_config(config_name='test-config.yml')
GlobalConfig().override_config(config_location=f"{os.path.dirname(os.path.realpath(__file__))}/../test-config.yml")
config = GlobalConfig().config

def test_simple_cluster_run():
Expand All @@ -21,7 +21,7 @@ def test_simple_cluster_run():
Job.insert('run', user_id=1, name=name, url=url, email=None, branch=branch, filename=filename, machine_id=machine_id)

ps = subprocess.run(
['python3', '../cron/client.py', '--testing', '--config-override', 'test-config.yml'],
['python3', '../cron/client.py', '--testing', '--config-override', f"{os.path.dirname(os.path.realpath(__file__))}/../test-config.yml"],
check=True,
stderr=subprocess.PIPE,
stdout=subprocess.PIPE,
Expand Down
16 changes: 8 additions & 8 deletions tests/cron/test_jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from lib.user import User
from tests import test_functions as Tests

GlobalConfig().override_config(config_name='test-config.yml')
GlobalConfig().override_config(config_location=f"{os.path.dirname(os.path.realpath(__file__))}/../test-config.yml")
config = GlobalConfig().config

# This should be done once per module
Expand All @@ -38,7 +38,7 @@ def get_job(job_id):

def test_no_run_job():
ps = subprocess.run(
['python3', '../cron/jobs.py', 'run', '--config-override', 'test-config.yml'],
['python3', '../cron/jobs.py', 'run', '--config-override', f"{os.path.dirname(os.path.realpath(__file__))}/../test-config.yml"],
check=True,
stderr=subprocess.PIPE,
stdout=subprocess.PIPE,
Expand All @@ -50,7 +50,7 @@ def test_no_run_job():

def test_no_email_job():
ps = subprocess.run(
['python3', '../cron/jobs.py', 'email', '--config-override', 'test-config.yml'],
['python3', '../cron/jobs.py', 'email', '--config-override', f"{os.path.dirname(os.path.realpath(__file__))}/../test-config.yml"],
check=True,
stderr=subprocess.PIPE,
stdout=subprocess.PIPE,
Expand Down Expand Up @@ -81,7 +81,7 @@ def test_simple_run_job_no_quota():
Job.insert('run', user_id=1, name=name, url=url, email=None, branch=branch, filename=filename, machine_id=machine_id)

ps = subprocess.run(
['python3', '../cron/jobs.py', 'run', '--config-override', 'test-config.yml'],
['python3', '../cron/jobs.py', 'run', '--config-override', f"{os.path.dirname(os.path.realpath(__file__))}/../test-config.yml"],
check=True,
stderr=subprocess.PIPE,
stdout=subprocess.PIPE,
Expand All @@ -108,7 +108,7 @@ def test_simple_run_job_quota_gets_deducted():
user.update()

ps = subprocess.run(
['python3', '../cron/jobs.py', 'run', '--config-override', 'test-config.yml'],
['python3', '../cron/jobs.py', 'run', '--config-override', f"{os.path.dirname(os.path.realpath(__file__))}/../test-config.yml"],
check=True,
stderr=subprocess.PIPE,
stdout=subprocess.PIPE,
Expand Down Expand Up @@ -156,7 +156,7 @@ def test_measurement_quota_exhausted():
user.deduct_measurement_quota(machine_id=machine_id, amount=2678400)

ps = subprocess.run(
['python3', '../cron/jobs.py', 'run', '--config-override', 'test-config.yml'],
['python3', '../cron/jobs.py', 'run', '--config-override', f"{os.path.dirname(os.path.realpath(__file__))}/../test-config.yml"],
check=True,
stderr=subprocess.PIPE,
stdout=subprocess.PIPE,
Expand All @@ -178,7 +178,7 @@ def test_machine_not_allowed():
user.update()

ps = subprocess.run(
['python3', '../cron/jobs.py', 'run', '--config-override', 'test-config.yml'],
['python3', '../cron/jobs.py', 'run', '--config-override', f"{os.path.dirname(os.path.realpath(__file__))}/../test-config.yml"],
check=True,
stderr=subprocess.PIPE,
stdout=subprocess.PIPE,
Expand Down Expand Up @@ -208,7 +208,7 @@ def todo_test_simple_email_job():
# Why is this patch not working :-(
with patch('email_helpers.send_email') as send_email:
ps = subprocess.run(
['python3', '../cron/jobs.py', 'email', '--config-override', 'test-config.yml'],
['python3', '../cron/jobs.py', 'email', '--config-override', f"{os.path.dirname(os.path.realpath(__file__))}/../test-config.yml"],
check=True,
stderr=subprocess.PIPE,
stdout=subprocess.PIPE,
Expand Down
3 changes: 2 additions & 1 deletion tests/lib/test_diff.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import os
from lib.db import DB
from lib.global_config import GlobalConfig
from tests import test_functions as Tests

GlobalConfig().override_config(config_name='test-config.yml')
GlobalConfig().override_config(config_location=f"{os.path.dirname(os.path.realpath(__file__))}/../test-config.yml")
config = GlobalConfig().config


Expand Down
Loading

0 comments on commit 98fce53

Please sign in to comment.