Run libev unit tests in CI and stop silently skipping tests#911
Merged
Conversation
📝 WalkthroughWalkthroughThis PR fixes a callback argument bug in Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Jun 29, 2026
sylwiaszunejko
force-pushed
the
enable-libev-unittest
branch
4 times, most recently
from
923c1e7 to
f453690
Compare
There was a problem hiding this comment.
Pull request overview
This PR makes CI test runs more trustworthy by preventing “silent skips” (where tests skip due to missing deps/config and CI still passes), and by making pytest output more verbose so skips are visible. It also includes a small bug fix in Session._set_keyspace_for_all_pools plus test adjustments to keep the shared libev reactor singleton in a working state for subsequent tests.
Changes:
- Add a
CASS_DRIVER_NO_SKIP-gated pytest hook to turn unexpected skips into failures, and enable it in cibuildwheel Linux/macOS test commands while explicitly deselecting known-incompatible tests. - Increase test visibility by adding
-vto pytest invocations and ensuringcompress-lz4is installed so lz4-related tests run in CI. - Fix keyspace-setting error aggregation in
Session._set_keyspace_for_all_poolsand adjust libev shutdown/reactor tests to avoid CI/import pitfalls and cross-test contamination.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/unit/io/test_libevreactor.py |
Restores libev singleton reactor state after cleanup to prevent downstream hangs. |
tests/unit/io/test_libevreactor_shutdown.py |
Adjusts subprocess import path precedence and improves subprocess failure diagnostics. |
tests/conftest.py |
Adds a no-skip CI mode by converting non-xfail skips into failures. |
pyproject.toml |
Enables no-skip + verbose pytest in cibuildwheel where extensions are required; installs lz4 extra for tests. |
cassandra/cluster.py |
Fixes _set_keyspace_for_all_pools to return aggregated errors rather than only the last pool’s errors. |
.github/workflows/integration-tests.yml |
Makes integration pytest runs verbose (-v) for better skip/failure visibility. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
test_watchers_are_finished calls libev__cleanup(), which stops the shared libev loop preparer. If a timer test runs later, the stopped preparer means timers are never scheduled and the test can hang. Restore _global_loop._shutdown and restart _global_loop._preparer after the cleanup path runs. Put the restoration in a finally block so the shared loop is left usable even if the post-cleanup watcher assertions fail.
Python 3.12 removed distutils from the standard library. Some Cython test helpers import pyximport, which still imports distutils through setuptools' compatibility shim during collection. Add setuptools to the dev test dependencies so those tests can collect in cibuildwheel's test environment.
Tests skip themselves when their requirements are missing (a library is absent, the wrong event loop is selected, the C extensions aren't built). That is convenient locally but a footgun in CI, where a test may be silently skipped because a dependency was not installed. The libev unit tests were effectively not running in any CI configuration. Add a CASS_DRIVER_NO_SKIP-gated pytest hook in tests/conftest.py that turns skips into failures (xfail untouched), and enable it in the cibuildwheel test-commands where the C extensions are mandatory (Linux/macOS). Tests that genuinely cannot run in the default configuration are listed explicitly via -k/--ignore (reactor tests run separately per EVENT_LOOP_MANAGER; asyncore, column_encryption and a few upstream-disabled/flaky tests excluded). Add -v to every pytest invocation and install the compress-lz4 extra so the lz4 tests actually run. Pass --import-mode=append in the cibuildwheel pytest commands so the installed compiled wheel takes precedence on sys.path over the in-tree pure-Python cassandra source during wheel tests. Keep the global pytest addopts unchanged so local pytest runs keep their normal import behavior. Windows and PyPy keep no-skip off: the extensions are optional on Windows and are never built on PyPy (setup.py forces is_pypy to skip libev/cmurmur3/Cython), so their extension-dependent skips are legitimate. The PyPy override also drops the compress-lz4 extra (no prebuilt PyPy lz4 wheel), uses cross-shell quoting, and deselects the known PyPy/Windows/macOS-incompatible tests.
The final callback was invoked with host_errors (the errors from only the last pool to finish) instead of the accumulated errors dict. If the last pool succeeded, failures from other pools were silently lost. Pass the aggregated errors dict, matching the method's docstring.
The atexit subprocess test inserted the project root at sys.path[0], which shadows the installed compiled wheel with the in-tree pure-Python source. Under cibuildwheel that source lacks the libev C extension, so the import failed and the subprocess produced no output. Append the project path instead so the installed wheel takes precedence, falling back to the source tree only when the driver is not installed. Also assert the subprocess return code and include stdout/stderr in the failure message so future subprocess import/runtime failures are easier to diagnose.
sylwiaszunejko
force-pushed
the
enable-libev-unittest
branch
from
f453690 to
bd81cd9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pyproject.toml (1)
161-180: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
&&chaining hides later reactor-suite results
test-commandentries are run sequentially with&&, so a failure in the main unit-test run stops the gevent/asyncio/eventlet commands from running in that CI attempt. If you want full signal from one job, run them as separate steps or accumulate exit codes instead.🤖 Prompt for 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. In `@pyproject.toml` around lines 161 - 180, The test-command in pyproject.toml currently relies on sequential execution, so a failure in the main unit-test invocation prevents the reactor-specific runs from executing. Update the test-command entries so the gevent, asyncio, and eventlet suites still run and report results even when an earlier command fails, either by splitting them into separate CI steps or by using an exit-code accumulation approach; keep the existing test-command grouping and CASS_DRIVER_NO_SKIP=1 / EVENT_LOOP_MANAGER setup intact.
🤖 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.
Nitpick comments:
In `@pyproject.toml`:
- Around line 161-180: The test-command in pyproject.toml currently relies on
sequential execution, so a failure in the main unit-test invocation prevents the
reactor-specific runs from executing. Update the test-command entries so the
gevent, asyncio, and eventlet suites still run and report results even when an
earlier command fails, either by splitting them into separate CI steps or by
using an exit-code accumulation approach; keep the existing test-command
grouping and CASS_DRIVER_NO_SKIP=1 / EVENT_LOOP_MANAGER setup intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a63b946-ca4f-45bf-8cb3-10fdcd36c208
📒 Files selected for processing (6)
.github/workflows/integration-tests.ymlcassandra/cluster.pypyproject.tomltests/conftest.pytests/unit/io/test_libevreactor.pytests/unit/io/test_libevreactor_shutdown.py
dkropachev
approved these changes
Jul 2, 2026
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
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.
Motivation
Tests skip themselves when their requirements are missing (a library is absent,
the wrong event loop is selected, the C extensions aren't built). That's fine
locally but a footgun in CI: a test can be silently skipped and nobody
notices. The libev unit tests were effectively not running in any CI
configuration — enabling them also surfaced two pre-existing
cluster.pybugs.Changes
CASS_DRIVER_NO_SKIP-gated pytest hook intests/conftest.pyturns skips into failures (xfail untouched). Enabled in thecibuildwheel test-commands where C extensions are mandatory (Linux/macOS).
Tests that genuinely can't run in the default config are listed explicitly via
-k/--ignore(reactor tests run separately perEVENT_LOOP_MANAGER;asyncore, column_encryption, and a few upstream-disabled/flaky tests excluded).
Windows and PyPy keep it off (the C extensions are optional on Windows and are
never built on PyPy, so their extension-dependent skips are legitimate). Local
dev is unaffected.
-vto all pytest invocations, and install thecompress-lz4test-extra so the lz4 tests actually run.Session._set_keyspace_for_all_poolsreported only the last pool's errorsinstead of the aggregated dict; failures from other pools were lost.
Session.wait_for_schema_agreementnow validatesscopeand raisesValueErrorfor unknown values (as the docstring already promised).Fixes: #904
Pre-review checklist
./docs/source/.Fixes:annotations to PR description.