drivaskbk01 commented on code in PR #43004:
URL: https://github.com/apache/superset/pull/43004#discussion_r4124244643
##########
.github/workflows/bashlib.sh:
##########
@@ -368,6 +383,108 @@ playwright-run() {
return $status
}
+playwright-run-gaq() {
+ # Global Async Queries needs more than a feature flag: submissions are handed
+ # to Celery, so without a worker consuming the queue the API returns 202 and
+ # no job ever runs -- the specs would time out rather than fail usefully.
+ # `playwright-run` boots gunicorn with this step's environment, so the flag
+ # set on the step reaches both the web server and the worker started here.
+ local APP_ROOT=$1
+ shift || true
+ local TEST_PATHS=("$@")
+
+ cd "$GITHUB_WORKSPACE"
+
+ if [ "${SUPERSET_FEATURE_GLOBAL_ASYNC_QUERIES:-}" != "true" ]; then
+ echo "::error::SUPERSET_FEATURE_GLOBAL_ASYNC_QUERIES must be \"true\" for
this step; the specs would skip themselves and report nothing."
+ return 1
+ fi
+
+ local workerlog="${HOME}/superset-gaq-worker.log"
+ say "::group::Start Celery worker for GAQ"
+ # Mirrors docker/docker-bootstrap.sh's worker invocation.
+ nohup celery --app=superset.tasks.celery_app:app worker \
+ -O fair \
+ --loglevel=INFO \
+ --concurrency=2 \
+ >"$workerlog" 2>&1 </dev/null &
+ local workerPid=$!
+
+ # Fail fast on a worker that never comes up: a dead worker is
+ # indistinguishable from a slow one once the specs start timing out.
+ local timeout=60
+ while [ $timeout -gt 0 ]; do
+ if ! kill -0 "$workerPid" 2>/dev/null; then
+ echo "::error::Celery worker exited during startup"
+ cat "$workerlog" || true
+ say "::endgroup::"
+ return 1
+ fi
+ if grep -q "celery@.*ready" "$workerlog" 2>/dev/null; then
+ say "Celery worker is ready"
+ break
+ fi
+ sleep 1
+ timeout=$((timeout - 1))
+ done
+ if [ $timeout -eq 0 ]; then
+ echo "::error::Celery worker failed to become ready within 60 seconds"
+ cat "$workerlog" || true
+ kill "$workerPid" 2>/dev/null || true
+ say "::endgroup::"
+ return 1
+ fi
+ say "::endgroup::"
+
+ # The GAQ specs are excluded from every other project, so this is what makes
+ # them loadable at all -- see the chromium-gaq project in
playwright.config.ts.
+ export INCLUDE_GAQ=true
+
+ local
report="${GITHUB_WORKSPACE}/superset-frontend/playwright-gaq-report.json"
+ rm -f "$report"
+ export PLAYWRIGHT_JSON_OUTPUT_NAME="$report"
+ # --workers=1: the fixtures all create charts as the same admin user, and
+ # Superset's tag listener races on the shared `editor:<id>` tag (see the
+ # chromium-gaq project in playwright.config.ts). That project's
+ # `fullyParallel: false` only orders tests within one file -- Playwright
+ # still runs separate files concurrently -- and this suite spans three, so
+ # one worker is what actually serializes it.
+ export PLAYWRIGHT_EXTRA_ARGS="--reporter=list,json --workers=1"
+
+ # `set -e` is on: without the guard a failing run would exit before the
+ # worker log is emitted and before the did-it-actually-run check below.
+ local status=0
+ playwright-run "$APP_ROOT" "${TEST_PATHS[@]}" || status=$?
+
+ unset PLAYWRIGHT_EXTRA_ARGS PLAYWRIGHT_JSON_OUTPUT_NAME INCLUDE_GAQ
+
+ say "::group::Celery worker log"
+ cat "$workerlog" || true
+ say "::endgroup::"
+ kill "$workerPid" 2>/dev/null || true
+
+ # A suite that skips itself still exits 0. That is the failure mode this step
+ # exists to prevent, so assert that tests actually ran.
+ if [ ! -f "$report" ]; then
+ echo "::error::No Playwright JSON report produced; cannot confirm the GAQ
specs ran."
+ return 1
+ fi
+ local expected skipped
+ expected=$(jq '.stats.expected // 0' "$report")
+ skipped=$(jq '.stats.skipped // 0' "$report")
+ say "GAQ suite: ${expected} passed, ${skipped} skipped"
+ if [ "$expected" -eq 0 ]; then
Review Comment:
Good catch — fixed in c9dcafb8af.
The gate now counts `expected + unexpected + flaky` as "executed" and logs
the breakdown, so a run where the worker loses Redis reports the real failure
instead of accusing GLOBAL_ASYNC_QUERIES of being inactive. The zero-skip
assertion is unchanged.
The line reads like this on the current green run:
```
GAQ suite: 10 passed, 0 failed, 0 flaky, 0 skipped
```
##########
.github/workflows/superset-playwright.yml:
##########
@@ -210,8 +210,142 @@ jobs:
${{ github.workspace }}/superset-frontend/test-results/
name: playwright-experimental-artifact-${{ github.run_id }}-${{
github.job }}-${{ matrix.browser }}--${{
steps.set-safe-app-root.outputs.safe_app_root }}
+ # GAQ runs in its own job rather than as a step in
playwright-tests-experimental
+ # above. A step with no explicit `if:` implicitly inherits `if: success()`,
so
+ # when GAQ was a step after Experimental/Mobile in that job, a failure in
+ # either of those unrelated suites skipped GAQ entirely rather than failing
it
+ # -- silently leaving that commit with zero GAQ coverage instead of a visible
+ # red check. A separate job can't share that fate: it either runs and reports
+ # for itself, or it doesn't start (e.g. the environment itself never came
up),
+ # which is the only case where "no GAQ result" is actually the right outcome.
+ playwright-tests-gaq:
+ needs: changes
+ if: needs.changes.outputs.python == 'true' ||
needs.changes.outputs.frontend == 'true'
+ runs-on: ubuntu-26.04
+ timeout-minutes: 30
+ continue-on-error: true
+ permissions:
+ contents: read
+ pull-requests: read
+ strategy:
+ fail-fast: false
+ matrix:
+ browser: ["chromium"]
+ app_root: ["", "/app/prefix"]
+ env:
+ SUPERSET_ENV: development
+ SUPERSET_CONFIG: tests.integration_tests.superset_test_config
+ SUPERSET__SQLALCHEMY_DATABASE_URI:
postgresql+psycopg2://superset:[email protected]:15432/superset
+ PYTHONPATH: ${{ github.workspace }}
+ REDIS_PORT: 16379
+ GITHUB_TOKEN: ${{ github.token }}
+ services:
+ postgres:
+ image: postgres:17-alpine
+ env:
+ POSTGRES_USER: superset
+ POSTGRES_PASSWORD: superset
+ ports:
+ - 15432:5432
+ redis:
+ image: redis:7-alpine
+ ports:
+ - 16379:6379
+ steps:
+ # -------------------------------------------------------
+ # Conditional checkout based on context (same as Cypress workflow)
+ - name: Checkout for push or pull_request event
+ if: github.event_name == 'push' || github.event_name == 'pull_request'
+ uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 #
v7.0.1
+ with:
+ persist-credentials: false
+ submodules: recursive
+ ref: ${{ github.event_name == 'pull_request' &&
github.event.pull_request.head.sha || github.sha }}
+ - name: Checkout using ref (workflow_dispatch)
+ if: github.event_name == 'workflow_dispatch' &&
github.event.inputs.ref != ''
+ uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 #
v7.0.1
+ with:
+ persist-credentials: false
+ ref: ${{ github.event.inputs.ref }}
+ submodules: recursive
+ - name: Checkout using PR ID (workflow_dispatch)
+ if: github.event_name == 'workflow_dispatch' &&
github.event.inputs.pr_id != ''
+ uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 #
v7.0.1
+ with:
+ persist-credentials: false
+ ref: refs/pull/${{ github.event.inputs.pr_id }}/merge
+ submodules: recursive
+ # -------------------------------------------------------
+ - name: Setup Python
+ uses: $/.github/actions/setup-backend/
+ - name: Setup postgres
+ uses: ./.github/actions/cached-dependencies
+ with:
+ run: setup-postgres
+ - name: Import test data
+ uses: ./.github/actions/cached-dependencies
+ with:
+ run: playwright_testdata
+ - name: Setup Node.js
+ uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 #
v7.0.0
+ with:
+ node-version-file: "./superset-frontend/.nvmrc"
+ cache: "npm"
+ cache-dependency-path: "superset-frontend/package-lock.json"
+ - name: Install npm dependencies
+ uses: ./.github/actions/cached-dependencies
+ with:
+ run: npm-install
+ - name: Build javascript packages
+ uses: ./.github/actions/cached-dependencies
+ with:
+ run: build-instrumented-assets
+ - name: Install Playwright
+ uses: ./.github/actions/cached-dependencies
+ with:
+ run: playwright-install
+ - name: Run Playwright (Global Async Queries Tests)
+ uses: ./.github/actions/cached-dependencies
+ env:
+ NODE_OPTIONS: "--max-old-space-size=4096"
+ # Scoped to this step for the same reason as the embedded and mobile
+ # flags in playwright-tests-experimental: GLOBAL_ASYNC_QUERIES
changes
+ # chart-data behaviour for every spec sharing a server.
+ #
+ # This suite has its own job, and is not required, because unlike the
+ # other flag-gated suites it needs a Celery worker: submissions are
+ # handed to Celery, so without one the API returns 202 and no job
ever
+ # executes. Putting a background worker and its startup window on the
+ # required path would let a GAQ flake block PRs that have nothing to
+ # do with async queries.
+ #
+ # `playwright-run-gaq` starts the worker and sets INCLUDE_GAQ, which
+ # is what makes the chromium-gaq project exist at all.
+ SUPERSET_FEATURE_GLOBAL_ASYNC_QUERIES: "true"
+ with:
+ run: >-
+ playwright-run-gaq "${{ matrix.app_root }}"
+ dashboard/global-async-query.spec.ts
Review Comment:
Fixed in c9dcafb8af. `playwright-run-gaq` now passes
`--project=chromium-gaq` and no spec paths, so the project's `testMatch` — the
same glob that excludes these specs from the default and sqllab projects — is
the single definition of what the suite contains. A new matching spec gets
picked up automatically instead of running nowhere.
`playwright-run`'s no-path branch now forwards `PLAYWRIGHT_EXTRA_ARGS` so
that works; it is empty for every other caller.
From the current green run:
```
Running with no explicit paths (project selection comes from
PLAYWRIGHT_EXTRA_ARGS, if set)
Running 10 tests using 1 worker
```
Note this thread shows as outdated because #44659 deleted
`superset-playwright.yml` while this branch was being merged up. The job moved
into `superset-e2e.yml` carrying this defect with it, so the fix landed there
rather than the thread being stale.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
