Skip to content

fix(tests): stop webhook render test comparing capped flow-run counts - #10285

Closed
saltas888 wants to merge 1 commit into
stablefrom
ds-fix-webhook-render-flowrun-page-cap
Closed

fix(tests): stop webhook render test comparing capped flow-run counts#10285
saltas888 wants to merge 1 commit into
stablefrom
ds-fix-webhook-render-flowrun-page-cap

Conversation

@saltas888

@saltas888 saltas888 commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Problem

backend/tests/functional/webhook/test_render.py::TestWebhookRender::test_branchless_event_triggers_webhook_process fails intermittently in CI, and once it starts failing in a given job it can never pass.

The test counted the webhook-process deployment's flow runs before and after posting the event:

runs_before = len(await prefect_client.read_flow_runs(deployment_filter=deployment_filter))
...
runs_after = len(await prefect_client.read_flow_runs(deployment_filter=deployment_filter))
assert runs_after > runs_before

That read is unsorted and passes no limit, so the Prefect server returns its default page — server.api.default_limit, which is 200. The Prefect server is session-scoped and accumulates webhook-process runs across the whole functional suite, so once the deployment has 200+ runs both len() calls pin at exactly 200 and runs_after > runs_before can never hold, regardless of whether the render under test worked.

The failure output shows precisely that — both sides of the comparison at 200:

E       AssertionError: webhook-process deployment was not run; server-side parameter render failed
E       assert > failed. [pytest-clarity diff shown]
E         200

This is not a regression from any recent change: the file is byte-identical on stable, develop and release-1.11, and the failure depends only on how many runs earlier tests left behind — which is why it presents as flakiness.

The package's own conftest.py already documents this exact hazard for the sibling webhook-send flow:

The session-scoped Prefect server accumulates webhook-send runs across the suite, and an unfiltered read is capped at the server's default page size. Sorting newest-first keeps a run a test just created within the returned page.

webhook-process never got the same treatment.

Fix

Read the run ids sorted newest-first and assert a new id appeared, so a full page no longer masks the new run:

runs = await prefect_client.read_flow_runs(
    deployment_filter=deployment_filter,
    sort=FlowRunSort.EXPECTED_START_TIME_DESC,
)
return {str(run.id) for run in runs}

The assertion keeps its original meaning (at least one new run). It deliberately does not tighten to "exactly one": this webhook is created with event_type="all", so unrelated activity in the session can legitimately trigger additional runs during the polling window — unlike the sibling only_new_run helper, whose webhook listens for a single event type.

The helper is kept local to the test file rather than moved into conftest.py, because the sibling helpers it mirrors do not exist in stable's conftest — this keeps the fix applying cleanly as it flows outward.

Base branch

Targets stable: the test file is byte-identical on stable, develop and release-1.11 (same blob), so fixing the oldest affected branch lets it reach the others through the normal syncs rather than needing three separate fixes.

Verification

  • The functional test passes locally against a live stack (uv run pytest backend/tests/functional/webhook/test_render.py — 1 passed, exit 0). A local run starts from a fresh Prefect server, so this exercises the under-200 path and confirms the fix does not break the passing case; the table below is what covers the failing one.

  • Simulated the cap for both implementations. Below it the old code works, which is why this passed historically; at and above it only the new code detects the run:

    existing runs old detects new run new detects new run
    5 yes yes
    199 yes yes
    200 no yes
    640 no yes
  • Confirmed server.api.default_limit == 200 in the installed Prefect, which is the cap the old code hit.

  • Confirmed against the installed client that read_flow_runs accepts both sort and deployment_filter, and that FlowRunSort.EXPECTED_START_TIME_DESC exists.

  • ruff check and ruff format --check clean on the changed file.

No changelog fragment: test-only change with no user-facing behaviour difference.

test_branchless_event_triggers_webhook_process counted the deployment's flow
runs before and after posting the event and asserted the count grew. The read
was unsorted and unlimited, so Prefect returned its default 200-row page:
once the webhook-process deployment accumulated 200 runs in a session, both
counts pinned at 200 and the assertion could never pass, failing the job
regardless of whether the render worked.

Read the run ids sorted newest-first and assert a new id appeared, so a full
page no longer masks the run. This mirrors the approach already documented for
webhook-send runs in the package conftest.
@github-actions github-actions Bot added the group/backend Issue related to the backend (API Server, Git Agent) label Aug 17, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

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 fix for a flaky assertion that compared capped flow-run counts; now reads sorted IDs and checks for new run IDs, with no production behavior change.

Re-trigger cubic

@codspeed-hq

codspeed-hq Bot commented Aug 17, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 13 untouched benchmarks


Comparing ds-fix-webhook-render-flowrun-page-cap (17c4222) with stable (bb98016)1

Open in CodSpeed

Footnotes

  1. No successful run was found on stable (982a6c0) during the generation of this report, so bb98016 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@fatih-acar

Copy link
Copy Markdown
Contributor

This should have been fixed in #10332

@fatih-acar fatih-acar closed this Aug 20, 2026
@fatih-acar
fatih-acar deleted the ds-fix-webhook-render-flowrun-page-cap branch August 20, 2026 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

group/backend Issue related to the backend (API Server, Git Agent)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants