Skip to content

ci(e2e): stop gunicorn worker recycling mid-run; slow-test budget for mixed-chart filter spec #42867

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sadpandajoe wants to merge 3 commits into
base: master
Choose a base branch
from
+17 −8
Open

ci(e2e): stop gunicorn worker recycling mid-run; slow-test budget for mixed-chart filter spec #42867

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b0bc04a
ci(e2e): stop recycling the single gunicorn worker mid-run
sadpandajoe Jul 31, 2026
7e772fd
test(dashboard): give the mixed-chart filter spec the slow-test budget
sadpandajoe Jul 31, 2026
f0c601c
Merge branch 'master' into pw-e2e-followups
sadpandajoe Aug 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions .github/workflows/bashlib.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,18 +201,23 @@ cypress-run-all() {
# navigation flow under E2E. We diverge from the entrypoint on:
# --timeout 120: heavy dashboard import/export specs exceed the 60s
# default
# --max-requests / --max-requests-jitter: recycle the worker under
# test load to avoid leaks accumulating across the run
# superset.app:create_app(): explicit factory so we don't depend on
# FLASK_APP being exported
#
# No --max-requests, matching the entrypoint's default of 0 (recycling
# off). With a single worker a recycle takes the whole backend offline for
# the graceful-timeout drain — browser keep-alive connections hold it open
# for the full 30s — plus ~5s of app boot. A run issues ~3800 requests in
# ~8 minutes, so recycling every 500 produced seven ~35s outages per run
# and flaked whichever specs happened to navigate into one. Lowering
# --graceful-timeout is not enough: a dashboard load plus chart render
# needs 6-10s, which still lands inside the window.
nohup gunicorn \
--bind "127.0.0.1:$port" \
--workers 1 \
--worker-class gthread \
--threads 20 \
--timeout 120 \
--max-requests 500 \
--max-requests-jitter 50 \
--access-logfile - \
--error-logfile - \
"superset.app:create_app()" \
Expand Down Expand Up @@ -294,16 +299,14 @@ playwright-run() {
export PLAYWRIGHT_BASE_URL

# See cypress-run-all() above for the args rationale (1 worker × 20
# gthread threads matching docker/entrypoints/run-server.sh, plus a
# 120s timeout and request-recycling for heavy E2E load).
# gthread threads matching docker/entrypoints/run-server.sh, a 120s
# timeout for heavy E2E load, and why worker recycling is off).
nohup gunicorn \
--bind "127.0.0.1:$port" \
--workers 1 \
--worker-class gthread \
--threads 20 \
--timeout 120 \
--max-requests 500 \
--max-requests-jitter 50 \
--access-logfile - \
--error-logfile - \
"superset.app:create_app()" \
Expand Down
6 changes: 6 additions & 0 deletions superset-frontend/playwright/tests/dashboard/mixed-chart-dashboard-filters.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
import { getDatasetByName } from '../../helpers/api/dataset';
import { extractIdFromResponse } from '../../helpers/api/assertions';
import { DashboardPage } from '../../pages/DashboardPage';
import { TIMEOUT } from '../../utils/constants';
import {
buildFilterJsonMetadata,
buildSelectFilter,
Expand All @@ -54,6 +55,11 @@ const FILTER_VALUE = 'boy';
testWithAssets(
'Mixed chart applies dashboard filter to both queries (#29519)',
async ({ page, testAssets }) => {
// Four API round-trips of setup precede a full dashboard load with a
// preselected native filter, matching the other dashboard specs that build
// their fixtures over the API rather than importing them.
testWithAssets.setTimeout(TIMEOUT.SLOW_TEST);

@codeant-ai-for-open-source codeant-ai-for-open-source Bot Aug 7, 2026

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.

Suggestion: Raising the overall test timeout does not extend the independent timeout used by DashboardPage.waitForChartsToLoad(), which defaults to TIMEOUT.API_RESPONSE (15 seconds). On a slow or cold CI runner, the dashboard can still fail during chart mounting/loading after 15 seconds even though the test now has a 60-second budget. Pass an appropriate longer timeout to the dashboard wait, or otherwise align the nested waits with the slow-test budget. [timeout and test-lifecycle behavior]

Severity Level: Major ⚠️ 
- ❌ Mixed-chart dashboard test can fail before its 60-second budget expires.
- ⚠️ Slow CI chart rendering causes avoidable flaky retries.
- ⚠️ Filter-propagation regression coverage becomes unreliable on cold runners.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** superset-frontend/playwright/tests/dashboard/mixed-chart-dashboard-filters.spec.ts
**Line:** 61:61
**Comment:**
	*Timeout And Test Lifecycle Behavior: Raising the overall test timeout does not extend the independent timeout used by `DashboardPage.waitForChartsToLoad()`, which defaults to `TIMEOUT.API_RESPONSE` (15 seconds). On a slow or cold CI runner, the dashboard can still fail during chart mounting/loading after 15 seconds even though the test now has a 60-second budget. Pass an appropriate longer timeout to the dashboard wait, or otherwise align the nested waits with the slow-test budget.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎


const dataset = await getDatasetByName(page, DATASET_NAME);
if (!dataset) {
throw new Error(`Dataset ${DATASET_NAME} not found`);
Expand Down
Loading