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

Parallelize tests #651

Draft
wants to merge 29 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
aeb8987
initial commit for parallization; introced parallel_id to Runner argu…
dan-mm Jan 11, 2024
f5d6cc0
changed conftest.py test cleanup to be per actual test session, not w…
dan-mm Jan 12, 2024
0057a4e
CI jobs to use parallel tests
dan-mm Jan 12, 2024
6d2d1b1
fix for test_uri_local_dir
dan-mm Jan 12, 2024
5c60e38
add parallel_id to networks; revert uri test fix; try different metho…
dan-mm Jan 12, 2024
30137e8
fix network check test
dan-mm Jan 15, 2024
66b2987
Merge branch 'main' into parallelize-tests
dan-mm Jan 16, 2024
ec5356f
- conftest cleanup now applies only after all workers finish
dan-mm Jan 16, 2024
e41731b
update pytest command for workflow
dan-mm Jan 18, 2024
d93d5d3
update workflow test command ;split test action into regular gmt test…
dan-mm Jan 18, 2024
148517c
changed to | tee -a so we can see the output
dan-mm Jan 18, 2024
d5a2fb8
WIP parallelize rewrite to not edit runner.py, but use parallel_id on…
dan-mm Jan 29, 2024
0cb02f4
remove parallel_id from runner; move custom loader and supporting fun…
dan-mm Feb 8, 2024
fa3e708
- updated pytest runstring; setup tests to use new test_functions wit…
dan-mm Feb 8, 2024
c160fac
smoke tests now use temp directory instead of stress-application dire…
dan-mm Feb 9, 2024
d5564dc
parallelize the tmp docker image
dan-mm Feb 9, 2024
3f4d7df
serialize failing test
dan-mm Feb 9, 2024
c9f28d5
removed leftover unneeded fstring
dan-mm Feb 9, 2024
a67cdf3
fix typo in test_volume_loading_subdirectories_root failing test (san…
dan-mm Feb 9, 2024
365bc6e
don't run examples directory tests
dan-mm Feb 9, 2024
693fe74
debug statement
dan-mm Feb 9, 2024
309a330
- found tests that were being run without no_build flag when they sho…
dan-mm Feb 13, 2024
eb537e7
corrected workflow input check syntax
dan-mm Feb 13, 2024
d860a33
capitalization
dan-mm Feb 13, 2024
5c8f3f0
github workflow inputs arent real bools
dan-mm Feb 13, 2024
0d8f26a
fix test_jobs (improper cleanup after insert job test); cleanup test …
dan-mm Feb 16, 2024
5cbc0b0
updated tests Readme
dan-mm Feb 16, 2024
d5087ed
removed unneeded dummy cpu util provider; renamed RUN_NAME -> name fo…
dan-mm Feb 16, 2024
c71ca90
Merge branch 'main' into parallelize-tests
dan-mm Feb 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions .github/actions/gmt-pytest/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@ inputs:
description: 'The root directory of the gmt repository'
required: false
default: '.'
tests-command:
description: 'The command to run the tests'
required: false
default: 'pytest'
run-examples-directory-tests:
description: 'Run tests for examples directory instead of regular gmt tests'
default: false
github-token:
description: 'pass in your secrets.GITHUB_TOKEN'
required: true
Expand Down Expand Up @@ -86,15 +85,22 @@ runs:
run: sleep 10s
shell: bash

# - name: Setup upterm session
# uses: lhotari/action-upterm@v1

- name: Run Tests
if: inputs.run-examples-directory-tests == 'false'
shell: bash
working-directory: ${{ inputs.gmt-directory }}/tests
run: |
source ../venv/bin/activate
python3 -m pytest -n auto -m "not serial" -rA | tee -a /tmp/test-results.txt
python3 -m pytest -m "serial" -rA | tee -a /tmp/test-results.txt

- name: Run Tests (examples directory)
if: inputs.run-examples-directory-tests == 'true'
shell: bash
working-directory: ${{ inputs.gmt-directory }}/tests
run: |
source ../venv/bin/activate
python3 -m ${{ inputs.tests-command }} -rA | tee /tmp/test-results.txt
python3 -m pytest ../../examples-directory/test/smoke_test.py -k "test_all_directories" -rA | tee -a /tmp/test-results.txt

- name: Display Results
shell: bash
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/tests-bare-metal-main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ jobs:
with:
metrics-to-turn-off: 'Machine Sensors Debug MacOS'
github-token: ${{ secrets.GITHUB_TOKEN }}
run-examples-directory-tests: false

- name: Eco CI Energy Estimation - Get Measurement
uses: green-coding-solutions/eco-ci-energy-estimation@v2
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/tests-eco-ci-energy-estimation.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ jobs:
with:
metrics-to-turn-off: '--categories RAPL Machine Sensors Debug CGroupV2 MacOS GPU --providers PsuEnergyAcSdiaMachineProvider'
github-token: ${{ secrets.GITHUB_TOKEN }}
run-examples-directory-tests: false

- name: Eco CI Energy Estimation - Get Measurement
uses: green-coding-solutions/eco-ci-energy-estimation@testing
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/tests-vm-main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ jobs:
with:
metrics-to-turn-off: '--categories RAPL Machine Sensors Debug CGroupV2 MacOS GPU --providers PsuEnergyAcSdiaMachineProvider'
github-token: ${{ secrets.GITHUB_TOKEN }}
run-examples-directory-tests: false

- name: Eco CI Energy Estimation - Get Measurement
uses: green-coding-solutions/eco-ci-energy-estimation@v2
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/tests-vm-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ jobs:
with:
metrics-to-turn-off: '--categories RAPL Machine Sensors Debug CGroupV2 MacOS GPU --providers PsuEnergyAcSdiaMachineProvider'
github-token: ${{ secrets.GITHUB_TOKEN }}
run-examples-directory-tests: false

- name: Eco CI Energy Estimation - Get Measurement
uses: green-coding-solutions/eco-ci-energy-estimation@v2
Expand Down
40 changes: 40 additions & 0 deletions lib/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
import string
import subprocess
import psycopg
import os
from pathlib import Path

from lib.db import DB

Expand Down Expand Up @@ -61,3 +63,41 @@ def get_architecture():
if output == 'darwin':
return 'macos'
return output

# This function takes a path and a file and joins them while making sure that no one is trying to escape the
# path with `..`, symbolic links or similar.
# We always return the same error message including the path and file parameter, never `filename` as
# otherwise we might disclose if certain files exist or not.
def join_paths(path, path2, mode='file'):
filename = os.path.realpath(os.path.join(path, path2))

# If the original path is a symlink we need to resolve it.
path = os.path.realpath(path)

# This is a special case in which the file is '.'
if filename == path.rstrip('/'):
return filename

if not filename.startswith(path):
raise ValueError(f"{path2} must not be in folder above {path}")

# To double check we also check if it is in the files allow list

if mode == 'file':
folder_content = [str(item) for item in Path(path).rglob("*") if item.is_file()]
elif mode == 'directory':
folder_content = [str(item) for item in Path(path).rglob("*") if item.is_dir()]
else:
raise RuntimeError(f"Unknown mode supplied for join_paths: {mode}")

if filename not in folder_content:
raise ValueError(f"{mode.capitalize()} '{path2}' not in '{path}'")

# Another way to implement this. This is checking the third time but we want to be extra secure 👾
if Path(path).resolve(strict=True) not in Path(path, path2).resolve(strict=True).parents:
raise ValueError(f"{mode.capitalize()} '{path2}' not in folder '{path}'")

if os.path.exists(filename):
return filename

raise FileNotFoundError(f"{path2} in {path} not found")
44 changes: 44 additions & 0 deletions lib/yml_helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#pylint: disable=too-many-ancestors

import yaml
import os
from lib import utils

class Loader(yaml.SafeLoader):
def __init__(self, stream):
# We need to find our own root as the Loader is instantiated in PyYaml
self._root = os.path.split(stream.name)[0]
super().__init__(stream)

def include(self, node):
# We allow two types of includes
# !include <filename> => ScalarNode
# and
# !include <filename> <selector> => SequenceNode
if isinstance(node, yaml.nodes.ScalarNode):
nodes = [self.construct_scalar(node)]
elif isinstance(node, yaml.nodes.SequenceNode):
nodes = self.construct_sequence(node)
else:
raise ValueError("We don't support Mapping Nodes to date")

filename = utils.join_paths(self._root, nodes[0], 'file')

with open(filename, 'r', encoding='utf-8') as f:
# We want to enable a deep search for keys
def recursive_lookup(k, d):
if k in d:
return d[k]
for v in d.values():
if isinstance(v, dict):
return recursive_lookup(k, v)
return None

# We can use load here as the Loader extends SafeLoader
if len(nodes) == 1:
# There is no selector specified
return yaml.load(f, Loader)

return recursive_lookup(nodes[1], yaml.load(f, Loader))

Loader.add_constructor('!include', Loader.include)
1 change: 1 addition & 0 deletions requirements-dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pylint==3.0.3
fastapi==0.109.2
starlette>=0.32
anybadge==1.14.0
pytest-xdist==3.5.0

# just to clear the pylint errors for the files in /api
scipy==1.12.0
Expand Down
89 changes: 4 additions & 85 deletions runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,52 +38,13 @@
from lib.global_config import GlobalConfig
from lib.notes import Notes
from lib import system_checks
from lib.yml_helpers import Loader

from tools.machine import Machine

def arrows(text):
return f"\n\n>>>> {text} <<<<\n\n"

# This function takes a path and a file and joins them while making sure that no one is trying to escape the
# path with `..`, symbolic links or similar.
# We always return the same error message including the path and file parameter, never `filename` as
# otherwise we might disclose if certain files exist or not.
def join_paths(path, path2, mode='file'):
filename = os.path.realpath(os.path.join(path, path2))

# If the original path is a symlink we need to resolve it.
path = os.path.realpath(path)

# This is a special case in which the file is '.'
if filename == path.rstrip('/'):
return filename

if not filename.startswith(path):
raise ValueError(f"{path2} must not be in folder above {path}")

# To double check we also check if it is in the files allow list

if mode == 'file':
folder_content = [str(item) for item in Path(path).rglob("*") if item.is_file()]
elif mode == 'directory':
folder_content = [str(item) for item in Path(path).rglob("*") if item.is_dir()]
else:
raise RuntimeError(f"Unknown mode supplied for join_paths: {mode}")

if filename not in folder_content:
raise ValueError(f"{mode.capitalize()} '{path2}' not in '{path}'")

# Another way to implement this. This is checking the third time but we want to be extra secure 👾
if Path(path).resolve(strict=True) not in Path(path, path2).resolve(strict=True).parents:
raise ValueError(f"{mode.capitalize()} '{path2}' not in folder '{path}'")

if os.path.exists(filename):
return filename

raise FileNotFoundError(f"{path2} in {path} not found")



class Runner:
def __init__(self,
name, uri, uri_type, filename='usage_scenario.yml', branch=None,
Expand Down Expand Up @@ -241,47 +202,7 @@ def checkout_repository(self):
# Inspiration from https://github.com/tanbro/pyyaml-include which we can't use as it doesn't
# do security checking and has no option to select when imported
def load_yml_file(self):
#pylint: disable=too-many-ancestors
class Loader(yaml.SafeLoader):
def __init__(self, stream):
# We need to find our own root as the Loader is instantiated in PyYaml
self._root = os.path.split(stream.name)[0]
super().__init__(stream)

def include(self, node):
# We allow two types of includes
# !include <filename> => ScalarNode
# and
# !include <filename> <selector> => SequenceNode
if isinstance(node, yaml.nodes.ScalarNode):
nodes = [self.construct_scalar(node)]
elif isinstance(node, yaml.nodes.SequenceNode):
nodes = self.construct_sequence(node)
else:
raise ValueError("We don't support Mapping Nodes to date")

filename = join_paths(self._root, nodes[0], 'file')

with open(filename, 'r', encoding='utf-8') as f:
# We want to enable a deep search for keys
def recursive_lookup(k, d):
if k in d:
return d[k]
for v in d.values():
if isinstance(v, dict):
return recursive_lookup(k, v)
return None

# We can use load here as the Loader extends SafeLoader
if len(nodes) == 1:
# There is no selector specified
return yaml.load(f, Loader)

return recursive_lookup(nodes[1], yaml.load(f, Loader))

Loader.add_constructor('!include', Loader.include)

usage_scenario_file = join_paths(self._folder, self._original_filename, 'file')
usage_scenario_file = utils.join_paths(self._folder, self._original_filename, 'file')

# We set the working folder now to the actual location of the usage_scenario
if '/' in self._original_filename:
Expand Down Expand Up @@ -563,8 +484,8 @@ def build_docker_images(self):
self.__notes_helper.add_note({'note': f"Building {service['image']}", 'detail_name': '[NOTES]', 'timestamp': int(time.time_ns() / 1_000)})

# Make sure the context docker file exists and is not trying to escape some root. We don't need the returns
context_path = join_paths(self._folder, context, 'directory')
join_paths(context_path, dockerfile, 'file')
context_path = utils.join_paths(self._folder, context, 'directory')
utils.join_paths(context_path, dockerfile, 'file')

docker_build_command = ['docker', 'run', '--rm',
'-v', f"{self._folder}:/workspace:ro", # this is the folder where the usage_scenario is!
Expand Down Expand Up @@ -671,7 +592,6 @@ def setup_services(self):
# If so, change the order of the services accordingly.
services_ordered = self.order_services(services)
for service_name, service in services_ordered.items():

if 'container_name' in service:
container_name = service['container_name']
else:
Expand Down Expand Up @@ -817,7 +737,6 @@ def setup_services(self):
docker_run_string.append('--net')
docker_run_string.append(self.__networks[0])


if 'pause-after-phase' in service:
self.__services_to_pause_phase[service['pause-after-phase']] = self.__services_to_pause_phase.get(service['pause-after-phase'], []) + [container_name]

Expand Down
25 changes: 19 additions & 6 deletions tests/README.MD
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,9 @@ run:

`python3 setup-test-env.py`

from the test directory. This will create a copy of the `config.yml` and docker `compose.yml` files that will be used in
from the test directory. This will create a copy of the docker `compose.yml` file that will be used in
the test containers. Please make sure that you have compiled all the metric providers and source code in lib. You can do
this automatically by using the `install.sh` command.

You will need to re-run this setup script if new metric providers are added or the config.yml is otherwise changed in a
significant way.
this automatically by using the `install_linux.sh` or `install_mac.sh` command.

## Running

Expand All @@ -42,9 +39,25 @@ There are a few scripts to make this easy.
`./run-tests.sh` will do everything - start the containers, run pytest, and then stop the containers.

The recommended workflow is to start the containers with the `./start-test-containers.sh` script, then in another shell
window run the pytest suite using `pytest`, and then stop the containers when your test run has finished.
window run the pytest suite using:

`pytest -n auto -m "not serial" --dist loadgroup && pytest -m "serial"`, and then stop the containers when your test run has finished.

Running a subset of tests using pytest is better explained within the documentation here:
https://docs.pytest.org/en/7.2.x/how-to/usage.html

You can also do everything in one command using the `./run-tests.sh` script.


## Parallelization
We now support running our test suite with parallelization using xdist. When writing tests it is important to note that not all tests can be parallelized, and the ones that cannot need to be marked accordingly. For parallelization, we use functions in test_functions.py to setup the environment with unique container names, as well as setting up the runner with setup_runner so that its tmp folders are also unique. If you bypass using the setup_runner, you will need need to still use the `parallelize_runner_folders` function to make sure its internal directories are correct.

Any test that cannot be parrallelized should be marked with:
`@pytest.mark.serial`

This includes any test that runs the runner through a subprocess, or otherwise creates a Runner class withhout using either test_functions.setup_runner or test_functions.parallelize_runner_folders

- tests that do not skip_system_checks can be parallelized, but only if they are marked with
`@pytest.mark.xdist_group(name="systems_checks")`

This will make all tests that use group name run sequentially on the same thread (but parallel to the rest of the suite). This is needed because we have a system check which makes sure the metric providers are not already running during setup.
Loading
Loading