drivaspreset commented on code in PR #43004:
URL: https://github.com/apache/superset/pull/43004#discussion_r4134999751
##########
.github/workflows/superset-e2e.yml:
##########
@@ -452,6 +452,174 @@ jobs:
fi
echo "cypress-matrix result: $RESULT (changes: $CHANGES)"
+ # GAQ runs in its own job rather than as a step in playwright-tests above.
+ # A step with no explicit `if:` implicitly inherits `if: success()`, so
+ # when GAQ was a step after the other suites 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_gaq
+ 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
Review Comment:
Good catch — fixed in d56f007ffc. Both services now use the GHCR mirrors,
matching every other job in this file:
```yaml
image: ghcr.io/apache/superset/ci/postgres:17-alpine
image: ghcr.io/apache/superset/ci/redis:7-alpine
```
Your point about the failure mode being invisible is the part that made this
worth fixing rather than leaving: on a `continue-on-error` job outside the
required check, a rate-limit hit at container init produces no signal to
anyone, so the suite would silently stop providing coverage while the PR still
looked green.
While in this job I also fixed two smaller inconsistencies against the
sibling `playwright-tests`: the step comment still referred to
`playwright-tests-experimental` (deleted by #44659), and "Set safe app root"
interpolated `${{ matrix.app_root }}` directly into the script where the
sibling passes it via `env:`.
--
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]
