Make backend-outage skips diagnosable - #97
Merged
Conversation
'155 skipped: backend outage' gave a reader nothing to debug with and no basis for deciding to ignore. The circuit breaker now records every event to a shared per-run log: the test that hit the backend first, with the URL of the failing call (pulled from the exception chain) and how it failed; each health probe's verdict (HTTP status or exception, with elapsed time); and every test the breaker fast-skipped. Skip messages carry the failing call or probe inline, and at session end the master aggregates the log into outage_report.md / outage_report.json. The existing skip report embeds the markdown — job summary, the neutral 'Run completeness' check, and the sticky PR comment all carry it, with the failing URLs as clickable links and the skipped-test list folded into a details block; the warning annotation gains a one-line digest. Verified end to end with a simulated trigger under the real conftest (during which the owl.virtualflybrain.org probe genuinely timed out and was named in the fast-skip message — the feature demonstrating itself).
'1 skipped' was as opaque as '155 skipped': the report only covered the circuit breaker, so an ordinary marker skip (tests/test_preview_warm.py's 'caching disabled: no patch layer to verify') stayed invisible without opening the Actions log. A pytest_runtest_logreport hook now records EVERY skip with its reason (xfails excluded), the report groups them by reason with per-group test lists in details folds, and the outage sections (first failing call with clickable URL, probe verdicts) sit on top when the breaker tripped. Renamed outage_report.* to skipped_tests_report.* to match, written whenever anything skipped.
The examples workflow's move to pytest (PR #96) broke it on Python 3.8: pytest resolves ignore::<class> filters at startup and exits 4 when the class does not exist, and that job's older marshmallow predates ChangedInMarshmallow4Warning. Both classes subclass DeprecationWarning, so filter DeprecationWarning from the marshmallow module instead — version-proof, and user-code DeprecationWarnings still show. Verified the suite still runs with zero marshmallow warnings leaking.
The Python 3.8 examples job failed with 'recorded number, live is int64': on that environment pandas hands counts back as numpy.int64, which the coarse type bucket did not recognise as a number, while the JSON recordings necessarily hold plain numbers. The environment's box type is not a schema property — numpy scalars register with the numbers ABCs (bool_/str_ matched by name), so int64 counts, float64 scores and numpy bools now land in the same bucket as their recorded JSON forms. A recorded number turning into a string still fails.
A nodeid like test_the_public_entry_point_reaches_the_decorated_original still forced the reader into the source tree to understand what went unverified. Every recorded skip and trigger now carries the test's location and the first line of its docstring; the report renders each test as a link to the exact file and line at the run's commit (when the GitHub Actions environment provides one) followed by that summary. Skips are recorded from the makereport wrapper — which holds the item — rather than a separate logreport hook.
test_the_public_entry_point_reaches_the_decorated_original skipped whenever VFBQUERY_CACHE_ENABLED=false — which the Run Tests workflow always sets, so CI never once verified the chain it exists to guard and the skip sat unexplained in every run (it was the '1 skipped'). Caching being off is a property of the test process, not of the code under test: when the ambient import ran unpatched, the same assertions now run in a subprocess that imports vfbquery with caching enabled — the real import-time patch path, production's default. No backend is contacted either way; the assertions only inspect bindings. The test now passes in both modes and never skips.
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.
What changed
"155 skipped: backend outage" — and equally "1 skipped" — gave a reader nothing to debug with and no basis for a decision to ignore. Whenever anything skips, the suite now writes
skipped_tests_report.md/.json: every skipped test with its reason, grouped by reason with per-group test lists in details folds — an ordinary marker skip shows up just like an outage skip. When the circuit breaker tripped, the outage detail sits on top: the test that hit the backend first with the URL of the failing call (clickable) and how it failed, and each health probe's verdict (HTTP status or exception, with elapsed time). Skip messages carry the failing call or probe inline, and the existing skip report embeds the markdown — job summary, the neutral "Run completeness" check and the sticky PR comment all carry it.Also rides along: the examples workflow broke on Python 3.8 after #96 because pytest errors at startup on
ignore::<class>filters naming a warning class that old marshmallow lacks — the filter is now category+module based (ignore::DeprecationWarning:marshmallow.*), version-proof, with user-code DeprecationWarnings still shown. The example shape comparison treats numpy scalars (int64 counts out of pandas on that environment) as numbers. And the report's first catch is fixed rather than documented:test_the_public_entry_point_reaches_the_decorated_originalskipped on every CI run because Run Tests pinsVFBQUERY_CACHE_ENABLED=false— it now verifies the caching patch chain in a subprocess with caching enabled when the ambient import ran unpatched, so it passes everywhere and never skips. Steady state for Run Tests is now 0 skipped.How to test
pytest tests/test_outage_reporting.py; for the full loop, run a file containing aConnectionError-raising test and a@pytest.mark.skiptest and read the generatedskipped_tests_report.md— both appear, grouped by reason, with the backend failure's call URL linked.