From ea54af414ea19c2a9ac9cf576b0a2590759648aa Mon Sep 17 00:00:00 2001 From: jnsbck <65561470+jnsbck@users.noreply.github.com> Date: Tue, 17 Dec 2024 15:13:34 +0100 Subject: [PATCH] Update Regression CI (#546) --- .github/workflows/regression_tests.yml | 124 ++++++++++++--- .../workflows/update_regression_baseline.yml | 141 ------------------ CHANGELOG.md | 6 +- tests/conftest.py | 30 ++++ tests/regression_test_baselines.json | 92 ------------ 5 files changed, 137 insertions(+), 256 deletions(-) delete mode 100644 .github/workflows/update_regression_baseline.yml delete mode 100644 tests/regression_test_baselines.json diff --git a/.github/workflows/regression_tests.yml b/.github/workflows/regression_tests.yml index c0e069d5..6b542839 100644 --- a/.github/workflows/regression_tests.yml +++ b/.github/workflows/regression_tests.yml @@ -1,23 +1,61 @@ -# .github/workflows/regression_tests.yml +# .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: Regression Tests on: - # pull_request: - # branches: - # - main + issue_comment: # trigger from comment; event runs on the default branch + types: [created] jobs: - regression_tests: - name: regression_tests + update_regression_tests: + name: update_regression_tests runs-on: ubuntu-20.04 + # Trigger from a comment that contains '/test_regression' + if: github.event.issue.pull_request && contains(github.event.comment.body, '/test_regression') + # 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: '## Regression Test\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; + - uses: actions/checkout@v3 with: + ref: main lfs: true - fetch-depth: 0 # This ensures we can checkout main branch too - - - uses: actions/setup-python@v4 + fetch-depth: 0 + + - name: Set up Python + uses: actions/setup-python@v4 with: python-version: '3.10' architecture: 'x64' @@ -26,14 +64,62 @@ jobs: run: | python -m pip install --upgrade pip pip install -e ".[dev]" - - - name: Run benchmarks and compare to baseline - if: github.event.pull_request.base.ref == 'main' + + - name: Update baseline + id: update-baseline + run: | + NEW_BASELINE=1 pytest -m regression + cp tests/regression_test_baselines.json /tmp/regression_test_baselines.json + + - 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 + + - name: Run comparison + id: comparison 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 \ No newline at end of file + cp /tmp/regression_test_baselines.json tests/regression_test_baselines.json + pytest -m regression + + - 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.comparison.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.comparison.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 + }); \ No newline at end of file diff --git a/.github/workflows/update_regression_baseline.yml b/.github/workflows/update_regression_baseline.yml deleted file mode 100644 index 4190da6c..00000000 --- a/.github/workflows/update_regression_baseline.yml +++ /dev/null @@ -1,141 +0,0 @@ -# .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_baseline' - if: github.event.issue.pull_request && contains(github.event.comment.body, '/update_regression_baseline') - # 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]" - - - 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 - }); \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 065c0577..54144e69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,9 @@ ```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. +- Add regression tests and supporting workflows for maintaining baselines (#475, #546, @jnsbck). + - Regression tests can be triggered by commenting on a PR. - 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. - refactor plotting (#539, @jnsbck). - rm networkx dependency diff --git a/tests/conftest.py b/tests/conftest.py index fd503c01..1caf1f33 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -206,6 +206,36 @@ def get_or_compute_swc2jaxley_params( params = {} +def pytest_collection_modifyitems(config, items): + if config.getoption("--runslow"): + # --runslow given in cli: do not skip slow tests + return + skip_slow = pytest.mark.skip(reason="need --runslow option to run") + for item in items: + if "slow" in item.keywords: + item.add_marker(skip_slow) + + +def pytest_collection_modifyitems(config, items): + NEW_BASELINE = ( + int(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") + + def should_skip_regression(): + return not NEW_BASELINE and not os.path.exists(baseline_fname) + + if should_skip_regression(): + for item in items: + if "regression" in item.keywords: + skip_regression = pytest.mark.skip( + reason="need NEW_BASELINE env to run" + ) + item.add_marker(skip_regression) + + @pytest.fixture(scope="session", autouse=True) def print_session_report(request, pytestconfig): """Cleanup a testing directory once we are finished.""" diff --git a/tests/regression_test_baselines.json b/tests/regression_test_baselines.json deleted file mode 100644 index 8d8483a7..00000000 --- a/tests/regression_test_baselines.json +++ /dev/null @@ -1,92 +0,0 @@ -{ - "ec3a4fad11d2bfb1bc5f8f10529cb06f2ff9919b377e9c0a3419c7f7f237f06e": { - "test_name": "test_runtime", - "input_kwargs": { - "num_cells": 1, - "artificial": false, - "connect": false, - "connection_prob": 0.0, - "voltage_solver": "jaxley.stone" - }, - "runtimes": { - "build_time": 0.5469788710276285, - "compile_time": 18.719950834910076, - "run_time": 2.8165321350097656 - } - }, - "128cfe30d4ffb9c1abd9dc0fa25b0e86940437b3eb1d46584e21f2c780ed78e8": { - "test_name": "test_runtime", - "input_kwargs": { - "num_cells": 1, - "artificial": false, - "connect": false, - "connection_prob": 0.0, - "voltage_solver": "jax.sparse" - }, - "runtimes": { - "build_time": 0.3167983690897624, - "compile_time": 3.1070560614267984, - "run_time": 2.4420499006907144 - } - }, - "45cb5fa937517154a8d7bd2ac6d4542ff66c7cd3f5199976706ae44134eec301": { - "test_name": "test_runtime", - "input_kwargs": { - "num_cells": 10, - "artificial": false, - "connect": true, - "connection_prob": 0.1, - "voltage_solver": "jaxley.stone" - }, - "runtimes": { - "build_time": 1.6278504530588787, - "compile_time": 29.477131684621174, - "run_time": 19.02045528093974 - } - }, - "872ba2d409d18daf5e0e947953385c3d0967087ed122f72ba01990397429318e": { - "test_name": "test_runtime", - "input_kwargs": { - "num_cells": 10, - "artificial": false, - "connect": true, - "connection_prob": 0.1, - "voltage_solver": "jax.sparse" - }, - "runtimes": { - "build_time": 1.2527097860972087, - "compile_time": 26.463435173034668, - "run_time": 25.086068153381348 - } - }, - "da2f14fe319cf40d2ec65fdde6f3e0c997ef803e637d1ae7d2f2846c2369dbb2": { - "test_name": "test_runtime", - "input_kwargs": { - "num_cells": 1000, - "artificial": true, - "connect": true, - "connection_prob": 0.001, - "voltage_solver": "jaxley.stone" - }, - "runtimes": { - "build_time": 110.89498209953308, - "compile_time": 45.626126766204834, - "run_time": 41.24793847401937 - } - }, - "4a250131b8b31132e19d9f82ececa4d2dc26b0678326089f8a2c9de3696418fc": { - "test_name": "test_runtime", - "input_kwargs": { - "num_cells": 1000, - "artificial": true, - "connect": true, - "connection_prob": 0.001, - "voltage_solver": "jax.sparse" - }, - "runtimes": { - "build_time": 108.67493462562561, - "compile_time": 60.997525453567505, - "run_time": 59.05851753552755 - } - } -} \ No newline at end of file