test(webhook): identify the render test's own flow run instead of counting - #10332
Merged
Conversation
Contributor
There was a problem hiding this comment.
No issues found across 3 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Test-only changes: fix flaky test by identifying runs by identity instead of count, add automation cleanup fixture, update testing docs.
Re-trigger cubic
fatih-acar
marked this pull request as ready for review
August 19, 2026 15:27
ajtmccarty
approved these changes
Aug 19, 2026
fatih-acar
force-pushed
the
fac-fix-webhook-render-flake
branch
from
August 19, 2026 21:26
693a05b to
d3d06ab
Compare
fatih-acar
marked this pull request as draft
August 19, 2026 21:26
…nting `read_flow_runs()` returns at most 200 rows (the Prefect API's default page size, and the API rejects a larger limit), so once the session's webhook-process runs fill that page the render test's before/after count comparison saturates at 200 and can never be true again. That makes the test fail deterministically whenever `TestWebhookConfigure` lands on the same xdist worker, which is how five unrelated PRs went red between Aug 13 and Aug 18 with a message blaming server-side parameter rendering. Identify the run by its id and webhook instead, and assert the rendered parameters really are the plain strings the test exists to guard. Delete the webhook automations a test class registers at its teardown: the Prefect server is session-scoped, so a surviving all-branches automation turned every event emitted by every later test in the worker into a scheduled webhook-process run that no worker ever executes. Measured over the webhook package plus one event-heavy ipam class: 216-243 leftover runs and 233-235s without the cleanup, 1 run and 182-201s with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fatih-acar
force-pushed
the
fac-fix-webhook-render-flake
branch
from
August 19, 2026 21:50
d3d06ab to
d5e654c
Compare
fatih-acar
marked this pull request as ready for review
August 20, 2026 07:35
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.
Why
test_render.py::TestWebhookRender::test_branchless_event_triggers_webhook_processfailed on 5unrelated PRs against
release-1.11between Aug 13 and Aug 18, always withAssertionError: webhook-process deployment was not run; server-side parameter render failed.Server-side rendering is fine — the test's own assertion is the bug, and it cannot pass under the
conditions CI now hits.
The test compared flow-run counts before and after posting an event:
read_flow_runs()returns at mostPREFECT_API_DEFAULT_LIMIT(200) rows, and the API rejects alarger
limitoutright (422 Invalid limit: must be less than or equal to 200). Once thewebhook-process deployment has >= 200 runs,
runs_before == runs_after == 200and the comparisoncan never be true. Every one of the five failures printed exactly
200, on four different runners.Those runs come from the webhook classes:
TestWebhookConfigure::test_configure_allcallsreconcile-all, and nothing deletes the automations afterwards. A surviving all-branches automation
turns every event any later test class emits into a scheduled webhook-process run (nothing executes
them — the functional suite runs no Prefect worker). Under
--dist loadscopethe test thereforefails iff a leaking webhook class landed on the same worker before it:
Non-goal: no production code changes. Nothing in
infrahub/is at fault here.What changed
test_render.pyidentifies the run it caused — newest-first read, filtered to run ids not seenbefore the event and to this webhook's
webhook_id— instead of counting rows, and then assertsthe rendered parameters are the plain strings the test exists to guard (
event_id,event_type,event_occured_at, an emptybranch_namefor the branch-less event, and the payload). Theprevious version only proved that some run appeared.
conftest.pygains an autouse class-scoped fixture that deleteswebhook::-prefixed automationsat class teardown. Automations outlive the class that created them while the webhook node behind
them is dropped with the class database.
dev/knowledge/backend/testing.mdrecords both traps under a new "Prefect Server State Outlivesthe Test Class" subsection.
How to review
The interesting question is whether run identity is robust where counting was not: a new
automation-triggered run is the newest by expected start time, so it lands at the top of the page
regardless of how many older runs exist, and the
webhook_idfilter keeps another automation's runfrom being mistaken for this one.
How to test
# The failing CI ordering, in one process: configure runs before render, same Prefect server uv run pytest backend/tests/functional/webhook -v29 passed locally in 2m47s, including the co-located ordering that fails in CI.
The 200-row cap was verified directly against a live Prefect test server: with 205 flow runs,
read_flow_runs()returns 200,limit=1000is a 422, andoffset=200reveals the remaining 5.The teardown makes the suite faster, not slower
Measured over
backend/tests/functional/webhookfollowed by one event-heavy class(
ipam/test_ipam_utilization.py) in a single process — the CI shape, where webhook classes runbefore event-heavy ones on the same worker — two pairs, alternating:
webhook::automationsThe cleanup is worth 33-53s (14-22%) on that slice, because the leaked automation generates
216-243 scheduled flow runs — each one a trigger evaluation plus rows in the same SQLite database
the tests are waiting on. It also reproduces the failure condition locally: a single event-heavy
class is enough to push the count past 200, and a CI worker runs ~15 of the suite's 62 classes.
The teardown call itself, timed against a live server (gather + deletes per class teardown):
Paging through all automations is cheap and flat (2-17ms even at 300). The cost is entirely in the
DELETEcalls, and it degrades badly past ~50 — 60ms each at 200, presumably trigger-servicereconfiguration serialized through SQLite. The webhook classes register 1-4 automations each, so
real cost is ~12-13ms per class teardown, ~0.1s across the package. Worth knowing if a future test
ever registers automations in bulk.
Impact & rollout
developandrelease-1.11carry a byte-identicaltest_render.py(same blob hash) and arevulnerable to the same failure, so they need this via the usual merge-forward. Other functional
suites (computed attributes, action rules) leave their own automations on the shared Prefect server
— same class of leak, different blast radius, deliberately out of scope here.
Checklist
user-facing
🤖 Generated with Claude Code