sadpandajoe commented on code in PR #43004:
URL: https://github.com/apache/superset/pull/43004#discussion_r4075359614
##########
superset-frontend/playwright.config.ts:
##########
@@ -161,6 +168,25 @@ export default defineConfig({
},
]
: []),
+ // Global Async Queries tests need the GLOBAL_ASYNC_QUERIES feature flag
+ // enabled in the Flask backend, plus Redis and a running Celery worker --
+ // without a worker, submissions return 202 and no job ever executes. The
+ // workflow's GAQ step provisions all three and sets INCLUDE_GAQ, so these
+ // specs never load in the required run, where the pipeline is inert. Same
+ // strict 'true' check as INCLUDE_EMBEDDED.
+ ...(process.env.INCLUDE_GAQ?.toLowerCase() === 'true'
+ ? [
+ {
+ name: 'chromium-gaq',
Review Comment:
The GAQ project still inherits `fullyParallel: true` and two CI workers, so
its dashboard fixtures race while creating the shared `editor:1` tag. Both
current GAQ matrix runs hit `tag_name_key`/`Chart could not be created` on
their first attempt and only passed on retry. Could this project serialize its
setup (for example, `fullyParallel: false`) so the coverage does not mask a
recurring flake?
##########
.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'
Review Comment:
This job only runs when the change detector reports Python or frontend
changes, but that detector does not match `superset-playwright.yml` itself or
composite-action setup paths. A PR that changes only this GAQ workflow can
therefore skip both GAQ legs and merge without exercising the changed job.
Could the condition include the relevant workflow/action paths?
--
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]
