Modernize testing - #134
Merged
Merged
Conversation
Splits test_dependent_estimates.py, which covered six modules, into the per-module files the rest of the suite already used: pymare.stats tests to test_stats.py, estimator tests to test_estimators.py, and so on. The cross-implementation checks against robumeta get their own module, so the job that regenerates their reference values can select them. Also brings the suite in line with how it is organized elsewhere: * pymare/tests is a package now, with helpers that are neither fixtures nor tests in utils.py and every fixture in conftest.py, including the ones that were defined inside test_results.py and test_effectsize_base.py. * The robumeta reference values move from a literal in a test module to data/robumeta_reference.json, written by validation/robumeta/run_robu.R. A pinned reference that a script regenerates can be diffed; a literal cannot. The values are bit-for-bit the ones they replace. * The Stan tests skip when pystan is missing instead of failing, and are marked so CI can give them a job with the extra installed. Their skipif(< 3.7) guards and the test whose body could only run on Python 3.6 are gone; python_requires has ruled both out for several releases. * python_requires and the classifiers move to 3.9-3.13. 3.8 is end-of-life, and the suite passes on 3.13 with numpy 2. * find: no longer sweeps benchmarks/ into the distribution. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t checks Four workflows, each mirrored by a make target so a check that passes locally is the one that runs on the pull request: * Run Tests. Unit tests over Python 3.9-3.13 on Linux and the ends of that range on macOS and Windows, with the Stan tests in their own job since compiling the model costs minutes. Coverage is uploaded per job and merged once, so a matrix cell no longer races the others to Codecov. Windows is new: the platform has not been covered before, so its first run may surface something. * Lint Code. flake8, split out of the test workflow, and now also over benchmarks/. * Benchmark. asv times the pull request against its base and fails the check once a benchmark is 1.3x slower with a statistically significant difference. The comparison is also posted as a pull request comment, updated in place; a fork's read-only token makes that step skip rather than the check pass. * Check robumeta alignment. Regenerates the pinned robumeta reference values from the pinned R image and fails if any number moved, then runs the alignment tests against what it just regenerated. This is what keeps the pin from quietly going stale, which a pinned file otherwise does. The asv suite covers the stats primitives, the estimators with and without group labels, the combination tests and the Dataset container. Sizes are set so one pass takes about four seconds, since the workflow times it twice per pull request. The threshold is loose on purpose: GitHub's shared runners are noisy enough that a tighter one would cry wolf. benchmarks/bench_cluster_robust.py stays a standalone report rather than becoming asv benchmarks; its shapes are far too large to time on every pull request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A manual run has no GITHUB_BASE_REF, so the fetch that names the two commits would have failed and the pull request comment had no pull request to go on. Also bumps the publish workflow's action versions and its Python to 3.11, which is inside the range the package now claims to support. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comparing master against a branch whose package code is byte-identical still reported four regressions of up to 1.7x, all of them on the combination tests -- the cheapest benchmarks in the suite, at a few milliseconds each, where one scheduler hiccup moves the measurement further than a real regression would. Gives them a larger second dimension so they time in tens of milliseconds, and raises the sample count for every benchmark, since the confidence interval asv compares is what decides significance. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GitHub disables a workflow with a schedule trigger once the repository has been quiet for 60 days, and a disabled workflow answers no trigger at all -- not push, not pull_request. That is why Run Tests did not run on this pull request: it was switched off on 2025-06-28 and editing the file kept its workflow id, so it kept its state. The three workflows added here are new paths, so they registered as new workflows and ran. Re-enabling is a repository toggle rather than a change to the file, but a disabled workflow cannot be re-run either, so recovering meant pushing a commit to get any run at all. workflow_dispatch removes that step next time. Added to both workflows that carry a cron, since both are exposed to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #134 +/- ##
==========================================
+ Coverage 91.10% 92.18% +1.08%
==========================================
Files 13 13
Lines 933 1817 +884
==========================================
+ Hits 850 1675 +825
- Misses 83 142 +59 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…comment The comment step 403'd on PR neurostuff#134. A pull request from a fork gets a read-only GITHUB_TOKEN, and the permissions block cannot elevate that -- it can only narrow what the token would otherwise have. CONTRIBUTING tells contributors to fork, so that step would have failed on very nearly every pull request, which makes it 35 lines of shell that reliably annotates the log in red and posts nothing. continue-on-error had kept the job green, so the alert itself was never at risk: a regression fails the check and the table lands in the job summary, both of which work from a fork. Dropping the comment leaves that path and removes the pull-requests: write permission, which is no longer needed. Not solved with pull_request_target on purpose: it would supply a writable token, but this workflow installs and executes the pull request's own code, so running it in the base repository's context would hand untrusted code a token and the repository secrets. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.