codeant-ai-for-open-source[bot] commented on code in PR #45696:
URL: https://github.com/apache/superset/pull/45696#discussion_r4369673629
##########
.github/workflows/superset-frontend.yml:
##########
@@ -109,7 +109,7 @@ jobs:
path: superset-frontend/coverage
report-coverage:
- needs: [sharded-jest-tests]
+ needs: [sharded-jest-tests] # This is required to configure all Jest tests
passed as required check in .asf.yaml
Review Comment:
Yes. `report-coverage` needs to run after a failed or skipped shard job and
then fail explicitly; `always()` alone would only make it run, not make it
fail. Also, because `frontend-build` is not listed in `needs`, its output
should not be referenced through the `needs` context here.
Add `frontend-build` as a direct dependency, and check the aggregate
matrix-job result before coverage-reporting steps:
```yaml
report-coverage:
needs: [frontend-build, sharded-jest-tests]
if: always() && needs.frontend-build.outputs.should-run == 'true'
steps:
- name: Require all Jest shards to pass
if: needs.sharded-jest-tests.result != 'success'
run: |
echo "::error::Jest shard job result: ${{
needs.sharded-jest-tests.result }}"
exit 1
# coverage-reporting steps follow
```
This makes the required check fail when the shard job fails, is cancelled,
or is skipped. If frontend tests are intentionally not run, the condition still
skips the coverage job. As written, the current change does not reliably
prevent a failing shard from merging.
--
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]
