Skip to content

Refactor test subsets in CI workflows #4788

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

Merged
merged 3 commits into from
Jun 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
89 changes: 80 additions & 9 deletions .github/workflows/pytest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,29 @@ on:
push:
branches: [main]


# Tests are split into multiple jobs to accelerate the CI.
# Different jobs should be organized to take approximately the same
# time to complete (and not be prohibitely slow).
# Because GitHub Actions don't support YAML anchors, we have to place the
# splitting of testfiles into groups in the strategy/matrix/test-subset
# and can't re-use the groups across jobs.
# A pre-commit hook (scripts/check_all_tests_are_covered.py)
# enforces that test run just once per OS / floatX setting.

jobs:
pytest:
ubuntu:
strategy:
matrix:
os: [ubuntu-18.04]
floatx: [float32, float64]
test-subset:
# Tests are split into multiple jobs to accelerate the CI.
# Different jobs should be organized to take approximately the same
# time to complete (and not be prohibitely slow)
#
# How this works:
# 1st block: Only passes --ignore parameters to pytest.
# → pytest will run all test_*.py files that are NOT ignored.
#
# Subsequent blocks: Only pass paths to test files.
# → pytest will run only these files
#
# Any test that was not ignored runs in the first job.
# A pre-commit hook (scripts/check_all_tests_are_covered.py)
# enforces that test run just once.
- |
--ignore=pymc3/tests/test_distributions_timeseries.py
--ignore=pymc3/tests/test_mixture.py
Expand Down Expand Up @@ -128,3 +131,71 @@ jobs:
env_vars: OS,PYTHON
name: codecov-umbrella
fail_ci_if_error: false
windows:
strategy:
matrix:
os: [windows-latest]
floatx: [float32, float64]
test-subset:
- |
pymc3/tests/test_distributions_random.py
- |
pymc3/tests/test_sampling.py
pymc3/tests/test_shared.py
- |
pymc3/tests/test_gp.py
pymc3/tests/test_ode.py
- |
pymc3/tests/test_model.py
pymc3/tests/test_model_func.py
pymc3/tests/test_modelcontext.py
pymc3/tests/test_pickling.py

fail-fast: false
runs-on: ${{ matrix.os }}
env:
TEST_SUBSET: ${{ matrix.test-subset }}
AESARA_FLAGS: floatX=${{ matrix.floatx }},gcc__cxxflags='-march=core2'
defaults:
run:
shell: cmd
steps:
- uses: actions/checkout@v2
- name: Cache conda
uses: actions/cache@v1
env:
# Increase this value to reset cache if conda-envs/environment-dev-py38.yml has not changed
CACHE_NUMBER: 0
with:
path: ~/conda_pkgs_dir
key: ${{ runner.os }}-conda-${{ env.CACHE_NUMBER }}-${{
hashFiles('conda-envs/windows-environment-dev-py38.yml') }}
- name: Cache multiple paths
uses: actions/cache@v2
env:
# Increase this value to reset cache if requirements.txt has not changed
CACHE_NUMBER: 0
with:
path: |
~/.cache/pip
$RUNNER_TOOL_CACHE/Python/*
~\AppData\Local\pip\Cache
key: ${{ runner.os }}-build-${{ matrix.python-version }}-${{
hashFiles('requirements.txt') }}
- uses: conda-incubator/setup-miniconda@v2
with:
activate-environment: pymc3-dev-py38
channel-priority: strict
environment-file: conda-envs/windows-environment-dev-py38.yml
use-only-tar-bz2: true # IMPORTANT: This needs to be set for caching to work properly!
- name: Install-pymc3
run: |
conda activate pymc3-dev-py38
pip install -e .
python --version
- name: Run tests
# This job uses a cmd shell, therefore the environment variable syntax is different!
# The ">-" in the next line replaces newlines with spaces (see https://stackoverflow.com/a/66809682).
run: >-
conda activate pymc3-dev-py38 &&
python -m pytest -vv --cov=pymc3 --cov-report=xml --cov-report term --durations=50 %TEST_SUBSET%
60 changes: 0 additions & 60 deletions .github/workflows/windows.yml

This file was deleted.

1 change: 1 addition & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ repos:
- repo: local
hooks:
- id: check-no-tests-are-ignored
additional_dependencies: [pandas,pyyaml]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no objection here, but pandas is a pretty heavy dependency for a pre-commit hook. I'll have a look at this later, perhaps it can be done without it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It certainly can be done with a bunch of dictionaries, but then the summarization doesn't work with one-liners and we'd have to do our own table printing. Not sure if that's worth the effort?

entry: python scripts/check_all_tests_are_covered.py
files: ^\.github/workflows/pytest\.yml$
language: python
Expand Down
101 changes: 78 additions & 23 deletions scripts/check_all_tests_are_covered.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,38 +6,93 @@
This is intended to be used as a pre-commit hook, see `.pre-commit-config.yaml`.
You can run it manually with `pre-commit run check-no-tests-are-ignored --all`.
"""
import itertools
import logging
import re
import os

from pathlib import Path

import pandas
import yaml

_log = logging.getLogger(__file__)
logging.basicConfig(level=logging.DEBUG)


if __name__ == "__main__":
testing_workflows = ["jaxtests.yml", "pytest.yml"]
ignored = set()
non_ignored = set()
for wfyml in testing_workflows:
pytest_ci_job = Path(".github") / "workflows" / wfyml
txt = pytest_ci_job.read_text()
ignored = set(re.findall(r"(?<=--ignore=)(pymc3/tests.*\.py)", txt))
non_ignored = non_ignored.union(set(re.findall(r"(?<!--ignore=)(pymc3/tests.*\.py)", txt)))
# Summarize
ignored_by_all = ignored.difference(non_ignored)
run_multiple_times = non_ignored.difference(ignored)
def find_testfiles():
dp_repo = Path(__file__).parent.parent
all_tests = {
str(fp.relative_to(dp_repo)).replace(os.sep, "/")
for fp in (dp_repo / "pymc3" / "tests").glob("**/test_*.py")
}
_log.info("Found %i tests in total.", len(all_tests))
return all_tests


def from_yaml():
"""Determins how often each test file is run per platform and floatX setting.

An exception is raised if tests run multiple times with the same configuration.
"""
# First collect the matrix definitions from testing workflows
matrices = {}
for wf in ["pytest.yml", "arviz_compat.yml", "jaxtests.yml"]:
wfname = wf.strip(".yml")
wfdef = yaml.safe_load(open(Path(".github", "workflows", wf)))
for jobname, jobdef in wfdef["jobs"].items():
matrix = jobdef.get("strategy", {}).get("matrix", {})
if matrix:
matrices[(wfname, jobname)] = matrix
else:
_log.warning("No matrix in %s/%s", wf, jobname)

# Now create an empty DataFrame to count based on OS/floatX/testfile
all_os = []
all_floatX = []
for matrix in matrices.values():
all_os += matrix["os"]
all_floatX += matrix["floatx"]
all_os = tuple(sorted(set(all_os)))
all_floatX = tuple(sorted(set(all_floatX)))
all_tests = find_testfiles()

df = pandas.DataFrame(
columns=pandas.MultiIndex.from_product(
[sorted(all_floatX), sorted(all_os)], names=["floatX", "os"]
),
index=pandas.Index(sorted(all_tests), name="testfile"),
)
df.loc[:, :] = 0

# Count how often the testfiles are included in job definitions
for matrix in matrices.values():
for os_, floatX, subset in itertools.product(
matrix["os"], matrix["floatx"], matrix["test-subset"]
):
testfiles = subset.split("\n")
ignored = {item.strip("--ignore=") for item in testfiles if item.startswith("--ignore")}
included = {item for item in testfiles if item and not item.startswith("--ignore")}
if ignored and not included:
# if no testfile is specified explicitly pytest runs all except the ignored ones
included = all_tests - ignored

for testfile in included:
df.loc[testfile, (floatX, os_)] += 1

ignored_by_all = set(df[df.eq(0).all(axis=1)].index)
run_multiple_times = set(df[df.gt(1).any(axis=1)].index)

# Print summary, warnings and raise errors on unwanted configurations
_log.info("Number of test runs (❌=0, ✅=once)\n%s", df.replace(0, "❌").replace(1, "✅"))

if ignored_by_all:
_log.warning(
f"The following {len(ignored_by_all)} tests are completely ignored: {ignored_by_all}"
)
_log.warning("%i tests are completely ignored:\n%s", len(ignored_by_all), ignored_by_all)
if run_multiple_times:
_log.warning(
f"The following {len(run_multiple_times)} tests are run multiple times: {run_multiple_times}"
raise Exception(
f"{len(run_multiple_times)} tests are run multiple times with the same OS and floatX setting:\n{run_multiple_times}"
)
if not (ignored_by_all or run_multiple_times):
print(f"✔ All tests will run exactly once.")
return


# Temporarily disabled as we're bringing features back for v4:
# assert not ignored_by_all
assert not run_multiple_times
if __name__ == "__main__":
from_yaml()