test: add smoke tests and CI for example scripts - #87
Conversation
PR Summary
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded subprocess tests for ten stochastic examples. The tests use deterministic seeds, reduced election counts, optional dependency checks, headless Matplotlib, output markers, and reference-table validation. CI runs the tests with Python 3.12. ChangesExample simulation validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #87 +/- ##
=======================================
Coverage 96.37% 96.37%
=======================================
Files 17 17
Lines 496 496
=======================================
Hits 478 478
Misses 18 18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_examples.py`:
- Around line 90-93: Update test_example_runs to validate each script’s
tabulated output rather than only asserting non-empty stdout. Define expected
table header or row patterns keyed by AFFECTED_SCRIPTS entries, then assert the
corresponding pattern appears in stdout after running _run_example, while
allowing unrelated lines such as timing output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a59ad76-02cf-4eb3-a6bf-d18dc7b0422c
📒 Files selected for processing (2)
.github/workflows/python-package.ymltests/test_examples.py
5136f56 to
2044bb9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_examples.py`:
- Around line 19-20: Update the module documentation in tests/test_examples.py
to list tabulate alongside joblib and matplotlib as a required optional examples
dependency, matching the module-level skip condition that checks tabulate
availability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a0d654d4-9a29-4b14-89c1-f1a1cd3aa1c3
📒 Files selected for processing (2)
.github/workflows/python-package.ymltests/test_examples.py
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/python-package.yml
|
Addressed CodeRabbit's smoke-test comment: |
452caad to
ee83a32
Compare
ee83a32 to
123bdf0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_examples.py`:
- Around line 169-175: Update the comparison logic around `comparable` to
iterate over every expected row except those in `cfg['skip']`, asserting each
exists in `actual` before value comparison. Validate that each corresponding row
has the same number of values as its expected row before comparing indexed
values, replacing the truncating `zip` behavior while preserving the existing
tolerance checks and error context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a9eb0cab-cf34-449d-87e4-a570e6142ffc
📒 Files selected for processing (2)
.github/workflows/python-package.ymltests/test_examples.py
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/python-package.yml
|
Strengthened the tests to check actual output values, not just completion: the six scripts with documented Typical result tables (merrill table_1/2/3/4, weber effectiveness, weber table_4) are now run on the real parallel path and every table cell is compared against the published reference (parsed from each script's docstring, so one source of truth). Tolerances are chosen so a legitimate run's MC error (~1 pp at 2,000–3,000 elections) never flakes, while an effective sample of only |
a4f15a5 to
a890057
Compare
|
We are also checking some against the values from the original papers while checking others against the typical output of the existing scripts, because mine never matched the original papers exactly. See #88 |
The example scripts are stochastic Monte Carlo verification scripts that were never executed by CI. Run each one in full (runpy) and read its computed table variable directly instead of parsing printed output; the tests are marked slow and skipped by default (pytest -m slow). For the six scripts with published reference results, compare the output values against hardcoded reference tables from the papers (REFERENCE_VALUES). Tolerances accommodate Monte Carlo error plus the scripts' measured deviations from the published tables; note that merrill_1984_table_2 and table_4 deviate from the papers by up to ~5 pp and ~9 pp respectively, so those two get looser tolerances and cannot detect an RNG-duplication regression the way the others do. The CI job runs the tests with pytest -m slow.
a890057 to
2275f30
Compare
1 participant
Background
The example scripts reproduce published voting-theory results (Merrill 1984, Weber 1977) and are excluded from the pytest suite (
--ignore=examples), so they are never executed in CI. #65 is about to refactor them for joblib parallelism; this PR establishes automated checks for them first, so the parallelization can be verified before and after.What this PR changes
tests/test_examples.py— runs each parallelized example at a reduced election count with an injected seed, asserting it completes and emits tabulated output; plus a reference-value check forweber_1977_effectiveness_tableagainst its documented results (within Monte Carlo tolerance). Skipped when joblib/matplotlib/tabulate aren't installed.ci: smoke-test-examplesjob inpython-package.yml— installsjoblib,matplotlib,tabulateand runs the new tests.Tests
The new smoke tests themselves (11 tests, ~1 min). CI for them is added in the same commit since it exists to run them (AGENTS.md).
Note: PR #65 (parallelization) is rebased on top of this branch.
Summary by CodeRabbit
Tests
Chores