Repository navigation
fix(ci): triage every failure and error in the marker-excluded suite, and make it report an executed count (#14930) - #14933
Merged
Merged
Conversation
This was referenced Aug 24, 2026
Contributor
✅ SSOT Configuration Compliance: Passing🎉 No new hardcoded values of either class — Known backlog in |
5 tasks
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.
Closes #14930
Thinking Path
The issue's corrected framing was right: the suite partially works. 35 marker-carrying
tests execute and pass. The question was what the other 38 results actually are, and
the answer is that 37 of 38 are one defect class in two disguises — a live service
the runner does not have, reported as a hard result instead of a stated non-result.
The 19 errors are not 19 problems. They are one file:
autobot-backend/services/redis_service_api_integration_test.py. It has nobackend_urlfixture anywhere on its conftest chain — the only definition lives inautobot-infrastructure/shared/tests/conftest.py, which this tree never reaches — soall 19 error at setup. Nothing else in the run raised that error.
The 19 failures are three causes, not one:
api/infrastructure_integration_test.pyConnectionRefuseddialling the backend on :8001api/voice_integration_test.pyConnectionRefuseddialling :8001 and the TTS worker on :8083knowledge/gpu_kb_integration_test.py,knowledge/kb_optimization_test.pyRuntimeError: Failed to initialize knowledge baseperformance_benchmarks_test.pyAssertionError: Memory usage too high—1644.55 < 500The 16 connection failures are the same shape as the 19 errors: real HTTP to services
that do not exist on a GitHub-hosted runner.
The two knowledge-base failures looked like the same thing and are not. The workflow
provides a healthy Redis specifically for these, and its header says so. They fail
anyway because
autobot-backend/conftest.pyreplacesautobot_shared.redis_clientwith a socket-free stand-in whose
get_redis_client()returnsNonefor everyitem under
autobot-backend/, gated on directory membership and not on markers. Sothe marker suite provisions a Redis that no backend test can reach — a service
configured, health-checked, and structurally unconsumable. That is filed as #14932
with the root cause and a proposed fix, and deliberately not fixed here: the change
flips Redis behaviour for every backend
integrationtest at once, and 35 of themcurrently pass. Landing it alongside the guard work would make the resulting count
unattributable.
The one genuine assertion defect is the benchmark. It asserted absolute process
RSS under 500 MB, but a pytest-xdist worker sits at ~1.6 GB from importing the
orchestrator, knowledge base, memory manager and LLM service before the first
benchmark runs. The budget was spent before the test started: it could not pass, and
it could not have caught a regression in the code it names.
On the decision the issue asks for — provide the fixture, or deselect. Both, and
they are not alternatives. The tests are well-formed and should run, so
backend_urlis defined rather than the module being marked out; but every requestgoes over the wire through a real
httpx.AsyncClient, so a live backend is a genuineprecondition.
knowledge_api_integration_test.pyis the contrast that settles it:it patches
aiohttp.ClientSession.get, needs only a syntactically valid URL string,and correctly just defines the fixture.
Nothing here narrows what the suite selects. Two tests assert that, on both the
scheduled expression and the dispatch default.
What Changed
autobot_shared/live_service_probe.py(new) — the canonical precondition probe.endpoint_is_listening()opens a TCP socket and catchesOSErrorand nothing wider,so "the service is absent" skips while a connected-but-wrong service, a malformed
target, or a programming error still fails.
redis_client_is_stubbed()detects thebackend stand-in structurally, by the absence of
__file__on an in-memoryModuleType, so the knowledge-base skips clear themselves the moment #14932 landsrather than being permanent by construction.
Four test modules now report absent services as skips naming the missing service,
via autouse fixtures — so a deselected test on the PR gate pays nothing. The
knowledge-base guards are scoped to
integration-marked items only: both files pairthe marked test with a hermetic one that runs on the PR gate, and guarding those too
would have silently disabled a passing gate test.
performance_benchmarks_test.pymeasures the RSS delta across the measuredregion. 500 MB of growth from one mocked
chat()call is a real leak; 500 MB ofabsolute RSS is AutoBot being imported.
pipeline-scripts/marker_suite_report.py(new) + the workflow's new Reportmarker coverage step. Both pytest invocations now write JUnit XML, and the script
aggregates collected / executed / passed / failed / errors / skipped into the run
summary and fails the job when the selection collected nothing. A missing report
file is a hard failure inside the script, never a zero — a pytest that died before
writing its XML must not read as "collected nothing", let alone as clean. The step
runs on
!cancelled()so a red pytest cannot hide the coverage count.The
|| [ $? -eq 5 ]lines stay, but no longer decide anything: they exist so thesecond invocation still runs and the report step still receives both files. Exit 5 is
now caught by the floor instead of being accepted.
Header. The file's triage record is written into it, and the stale "one observed
run is RED (11 failed, 7 passed, 40 errors)" paragraph is corrected.
Verification
Before, run
32702629146on PR #14929's head4d050db81:After, run
32713266629on headb3096ed86— the workflow is green, the firstgreen run in its history (every prior run, on any branch, was red):
and its own new report step, on the run summary page:
Every one of the 38 failures and errors became a skip with a stated reason, counted by
the new report step. From the run's own summary table and the uploaded JUnit artifact:
the AutoBot backend API is not listening on 127.0.0.1:8001Chromium is not downloaded for Playwright(pre-existing)the knowledge base needs a real Redis client … tracked as #14932collection skipped(pre-existing)AUTOBOT_SUMMARIZATION_PROVIDER is not set(pre-existing)(Counts above are from the intermediate run
32709235066; the final run's 49 skips arethe same set less the over-reach fixed below.)
19 + 6 + 10 + 2 = 37formerfailures/errors became skips. The 12 pre-existing skips are unchanged.
TestLLMServicePerformance::test_llm_response_time_benchmarknow passes — the RSSdelta fix — confirmed test-by-test in the artifact.
The two count deltas, both explained rather than waved at:
85 → 84 collected. Not a lost test. The "before" run was on #14929's head, which adds
tools scripts pipeline-scripts autobot-infrastructure/shared/scripts/hooksto thepytest roots; this branch is on base, which has four roots. #14929's own comment names
the extra test: "
scripts/check_script_exec_bits_test.py's@pytest.mark.integrationcase was selected by NO workflow at all." Nothing here removes a root, and that test
returns to the selection when #14929 lands.
35 → 34 passed — a real over-reach in this PR, found and fixed. The infrastructure
guard was module-wide, but only 6 of that module's 7 tests dial the backend;
test_celery_worker_statusreadslogs/celery-worker.logoff disk and was passing. The7-skips-vs-6-failures mismatch is what exposed it. It is now exempted by an explicit
constant that self-checks: the fixture asserts every exempted name still resolves in
the module, so a rename fails loudly instead of stranding the exemption silently. An AST
check confirms the exemption set equals exactly the tests containing no
requests.call.With that fix the measured figures are 35 passed, 49 skipped, 0 failed, 0 errors
(run
32713266629) — a net+1pass over base, from the benchmark repair.Full PR CI, deduped to the latest entry per check name: 50 SUCCESS, 1 SUCCESS
(StatusContext), 9 SKIPPED, 0 failing, 0 cancelled, 0 pending.
mergeStateStatusis
BLOCKEDonREVIEW_REQUIRED— the ruleset review gate, not a check.One further defect found and filed, not fixed: #14935. Run
32710575293went red ontest_system_startup_performanceat537.86msagainst a500.0msbudget, while therun before and after it passed — a boundary flake on runner contention. Underneath it is
a real bug: every
MultiModalProcessor()logsFailed to load vision models: CLIPModel.__init__() got an unexpected keyword argument 'resume_download'and continues, so no vision model loads at all and the benchmark is largely timing a
failing
transformerscall. Pre-existing (it passed by luck in the "before" run) andunrelated to this PR's subject, but it will redden this suite about half the time, so it
must be fixed before the cron in #13286 is enabled. Relaxing the budget to clear the
board is explicitly rejected there.
Mutation pairs. Seven mutations were applied to the workflow, each verified
present in the source before mutating so a missing target could not report a false
pass. All seven were caught:
Two of those exist because the exercise found real holes in this PR's own tests:
marker_suite_report.py, so renaming thescript in the workflow left no token matching and the loop passed vacuously. It
now asserts that every
.pypath the workflow references exists, which catches therename from either end.
MARKER_EXPRESSIONisinputs.markers || <default>, so two strings decide what runs and only one waspinned.
Two regressions this PR introduced and fixed before merge, both caught by CI rather
than assumed away:
marker_suite_report_test.pyfailed collection on all 12 python-suite shards.On Python 3.14
@dataclassresolves string annotations throughsys.modules[cls.__module__], which isNonefor a module executed without asys.modulesentry — the loader pattern this repo uses to avoid the sys.modules leakguard. Fixed at the source by dropping
from __future__ import annotationsfrom thescript, plus a register/
finally-pop loader so re-adding it cannot resurrect theerror. Verified against a scratchpad copy, including that the loader leaves no entry.
AUTOBOT_LIVE_PROBE_TIMEOUT_SECONDSwas unregistered. Added toautobot_shared/env_registry.pywith its range, and the autogenerated table indocs/developer/CLAUDE_RULES.mdregenerated — verified by rebuilding the whole147-row table from the registry via AST and diffing it against the committed section.
Presence assertions.
pipeline-scripts/marker_suite_report_test.pyfeeds thechecker the empty and absent cases and asserts it fails: zero collected, an all-skipped
run, a missing report file, malformed XML, a
<testsuite>missing an attribute, and--min-collected 0(rejected at the argument parser — the presence assertion is notdisableable from the call site).
autobot_shared/live_service_probe_test.pypins theprobe's boundary: a non-
OSErrorfromcreate_connectionpropagates rather thanreading as "absent", and an unparseable target raises instead of skipping a live suite.
No code from the codebase was executed locally; the mutation and wiring checks above
read YAML and re-implement the assertions rather than importing the repo.
Residuals, filed not dropped: #14932 — the conftest Redis stand-in, with the root
cause, the proposed one-line window-4 fix, and a note that the marker run also lacks
config/redis-databases.yamlso named databases never resolve; and #14935 above.Closesnames only #14930, which is fully delivered.Note for #14929: it also edits
marker-tests.yml(appending pytest roots) and willconflict with the restructured run steps here. Its roots re-apply cleanly onto the new
step bodies; nothing in this PR removes them.
Model Used
Claude Opus 5 (1M context)