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

Add runtime tests #475

Merged
merged 13 commits into from
Dec 2, 2024
39 changes: 39 additions & 0 deletions .github/workflows/regression_tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# .github/workflows/regression_tests.yml
name: Regression Tests
jnsbck marked this conversation as resolved.
Show resolved Hide resolved

on:
pull_request:
branches:
- main

jobs:
regression_tests:
name: regression_tests
runs-on: ubuntu-20.04

steps:
- uses: actions/checkout@v3
with:
lfs: true
fetch-depth: 0 # This ensures we can checkout main branch too

- uses: actions/setup-python@v4
with:
python-version: '3.10'
architecture: 'x64'

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install -e ".[dev]"
jnsbck marked this conversation as resolved.
Show resolved Hide resolved

- name: Run benchmarks and compare to baseline
if: github.event.pull_request.base.ref == 'main'
run: |
# Check if regression test results exist in main branch
if [ -f 'git cat-file -e main:tests/regression_test_baselines.json' ]; then
git checkout main tests/regression_test_baselines.json
else
echo "No regression test results found in main branch"
fi
pytest -m regression
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,4 @@ jobs:
- name: Test with pytest
run: |
pip install pytest pytest-cov
pytest tests/ --cov=jaxley --cov-report=xml
pytest tests/ -m "not regression" --cov=jaxley --cov-report=xml
141 changes: 141 additions & 0 deletions .github/workflows/update_regression_baseline.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
# .github/workflows/update_regression_tests.yml

# for details on triggering a workflow from a comment, see:
# https://dev.to/zirkelc/trigger-github-workflow-for-comment-on-pull-request-45l2
name: Update Regression Baseline

on:
issue_comment: # trigger from comment; event runs on the default branch
types: [created]

jobs:
update_regression_tests:
name: update_regression_tests
runs-on: ubuntu-20.04
# Trigger from a comment that contains '/update_regression_baselines'
if: github.event.issue.pull_request && contains(github.event.comment.body, '/update_regression_baselines')
# workflow needs permissions to write to the PR
permissions:
contents: write
pull-requests: write
issues: read

steps:
- name: Create initial status comment
uses: actions/github-script@v7
id: initial-comment
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
const response = await github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: '## Updating Regression Baselines\n⏳ Workflow is currently running...'
});
return response.data.id;

- name: Check if PR is from fork
id: check-fork
uses: actions/github-script@v7
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
const pr = await github.rest.pulls.get({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: context.issue.number
});
return pr.data.head.repo.fork;

- name: Get PR branch
uses: xt0rted/pull-request-comment-branch@v3
id: comment-branch
with:
repo_token: ${{ secrets.GITHUB_TOKEN }}

- name: Checkout PR branch
uses: actions/checkout@v3
with:
ref: ${{ steps.comment-branch.outputs.head_sha }} # using head_sha vs. head_ref makes this work for forks
lfs: true
fetch-depth: 0 # This ensures we can checkout main branch too

- uses: actions/setup-python@v4
with:
python-version: '3.10'
architecture: 'x64'

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install -e ".[dev]"
jnsbck marked this conversation as resolved.
Show resolved Hide resolved

- name: Update baseline
id: update-baseline
run: |
git config --global user.name '${{ github.event.comment.user.login }}'
git config --global user.email '${{ github.event.comment.user.login }}@users.noreply.github.com'
# Check if regression test results exist in main branch
if [ -f 'git cat-file -e main:tests/regression_test_baselines.json' ]; then
git checkout main tests/regression_test_baselines.json
else
echo "No regression test results found in main branch"
fi
NEW_BASELINE=1 pytest -m regression

# Pushing to the PR branch does not work if the PR is initiated from a fork. This is
# because the GITHUB_TOKEN has read-only access by default for workflows triggered by
# fork PRs. Hence we have to create a new PR to update the baseline (see below).
- name: Commit and push to PR branch (non-fork)
# Only run if baseline generation succeeded
if: success() && steps.update-baseline.outcome == 'success' && !fromJson(steps.check-fork.outputs.result)
run: |
git add -f tests/regression_test_baselines.json # since it's in .gitignore
git commit -m "Update regression test baselines"
git push origin HEAD:${{ steps.comment-branch.outputs.head_ref }} # head_ref will probably not work for forks!

- name: Create PR with updates (fork)
if: success() && steps.update-baseline.outcome == 'success' && fromJson(steps.check-fork.outputs.result)
uses: peter-evans/create-pull-request@v5
id: create-pr
with:
token: ${{ secrets.GITHUB_TOKEN }}
commit-message: Update regression test baselines
title: 'Update regression test baselines'
branch: regression-baseline-update-${{ github.event.issue.number }}
base: ${{ steps.comment-branch.outputs.head_ref }}

- name: Update comment with results
uses: actions/github-script@v7
if: always() # Run this step even if previous steps fail
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
const fs = require('fs');
let status = '${{ steps.update-baseline.outcome }}' === 'success' ? '✅' : '❌';
let message = '## Regression Baseline Update\n' + status + ' Process completed\n\n';

try {
const TestReport = fs.readFileSync('tests/regression_test_report.txt', 'utf8');
message += '```\n' + TestReport + '\n```\n\n';

// Add information about where the changes were pushed
if ('${{ steps.update-baseline.outcome }}' === 'success') {
if (!${{ fromJson(steps.check-fork.outputs.result) }}) {
message += '✨ Changes have been pushed directly to this PR\n';
} else {
const prNumber = '${{ steps.create-pr.outputs.pull-request-number }}';
message += `✨ Changes have been pushed to a new PR #${prNumber} because this PR is from a fork\n`;
}
}
} catch (error) {
message += '⚠️ No test report was generated\n';
}

await github.rest.issues.updateComment({
comment_id: ${{ steps.initial-comment.outputs.result }},
owner: context.repo.owner,
repo: context.repo.repo,
body: message
});
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ coverage.xml
*.py,cover
.hypothesis/
.pytest_cache/
tests/regression_test_results.json
tests/regression_test_baselines.json
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't get this. Why do we first put it in the gitignore just to then force add it?

Copy link
Contributor

@jnsbck jnsbck Dec 2, 2024

Choose a reason for hiding this comment

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

I agree this appears a bit odd, but I want to prevent users from pushing an updated baseline file from their local machine (which is why I have added it to the gitignore). The only way baselines should be updated is through github actions (to ensure regression tests are run in a consistent environment). That is why the github actions uses -f, while for the user changes to these files are ignored.

EDIT: This also makes running regression tests locally easier, since: running NEW_BASELINE=1 pytest -m regression on main and then pytest -m regression on feature. Ignores the changes to the baseline when switching the branch, so no stashing etc. necessary to run the test on feature with the baseline file from main.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does that make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, got it, yes makes sense!


# Translations
*.mo
Expand Down
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
```python
net.record("i_IonotropicSynapse")
```

- Add regression tests and supporting workflows for maintaining baselines (#475, @jnsbck).
- PRs now trigger both tests and regression tests.
- Baselines are maintained in the main branch.
- Regression tests can be done locally by running `NEW_BASELINE=1 pytest -m regression` i.e. on `main` and then `pytest -m regression` on `feature`, which will produce a test report (printed to the console and saved to .txt).
- If a PR introduces new baseline tests or reduces runtimes, then a new baseline can be created by commenting "/update_regression_baselines" on the PR.

# 0.5.0

Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ dev = [
[tool.pytest.ini_options]
markers = [
"slow: marks tests as slow (T > 10s)",
"regression: marks regression tests",
]

[tool.isort]
Expand Down
43 changes: 43 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# This file is part of Jaxley, a differentiable neuroscience simulator. Jaxley is
# licensed under the Apache License Version 2.0, see <https://www.apache.org/licenses/>

import json
import os
from copy import deepcopy
from typing import Optional
Expand All @@ -9,6 +10,7 @@

import jaxley as jx
from jaxley.synapses import IonotropicSynapse
from tests.test_regression import generate_regression_report, load_json


@pytest.fixture(scope="session")
Expand Down Expand Up @@ -202,3 +204,44 @@ def get_or_compute_swc2jaxley_params(

yield get_or_compute_swc2jaxley_params
params = {}


@pytest.fixture(scope="session", autouse=True)
def print_session_report(request, pytestconfig):
"""Cleanup a testing directory once we are finished."""
NEW_BASELINE = os.environ["NEW_BASELINE"] if "NEW_BASELINE" in os.environ else 0

dirname = os.path.dirname(__file__)
baseline_fname = os.path.join(dirname, "regression_test_baselines.json")
results_fname = os.path.join(dirname, "regression_test_results.json")

collected_regression_tests = [
item for item in request.session.items if item.get_closest_marker("regression")
]

def update_baseline():
if NEW_BASELINE:
results = load_json(results_fname)
with open(baseline_fname, "w") as f:
json.dump(results, f, indent=2)
os.remove(results_fname)

def print_regression_report():
baselines = load_json(baseline_fname)
results = load_json(results_fname)

report = generate_regression_report(baselines, results)
# "No baselines found. Run `git checkout main;UPDATE_BASELINE=1 pytest -m regression; git checkout -`"
with open(dirname + "/regression_test_report.txt", "w") as f:
f.write(report)

# the following allows to print the report to the console despite pytest
# capturing the output and without specifying the "-s" flag
capmanager = request.config.pluginmanager.getplugin("capturemanager")
with capmanager.global_and_fixture_disabled():
print("\n\n\nRegression Test Report\n----------------------\n")
print(report)

if len(collected_regression_tests) > 0:
request.addfinalizer(update_baseline)
request.addfinalizer(print_regression_report)
Loading
Loading