You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/07/28 05:05:43 UTC

[PR] feat(ci): publish Python unit test results as PR check annotations [superset]

rusackas opened a new pull request, #42503:
URL: https://github.com/apache/superset/pull/42503

   ### SUMMARY
   Right now a red check on Python-Unit just means scrolling raw pytest logs to find which test broke. This wires up EnricoMi/publish-unit-test-result-action so failing tests get annotated inline on the PR diff instead.
   
   pytest already writes JUnit XML for free via `--junit-xml`, so I added that to all three pytest invocations in the `unit-tests` job (the main suite and the two 100%-coverage sub-suites) and upload the results as an artifact, even on failure since that's exactly when it matters. A second workflow publishes the check run, triggered via `workflow_run` so it always runs in the base-branch context and can safely hold `checks:write` even for fork or Dependabot PRs, without ever checking out PR code. Same pattern this repo already uses for the translation-regression bot.
   
   Scoped to Python-Unit only for now. Jest's 8-way shard and the Python integration matrix (3 DBs + presto/hive) seem like natural follow-ups once this pattern proves out, figured it's better to land something small first.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   N/A, CI-only change.
   
   ### TESTING INSTRUCTIONS
   `zizmor` and `pre-commit` both pass clean on the changed/new workflow files.
   
   One thing worth flagging: the second workflow won't actually fire on this PR itself. `workflow_run` only triggers once both files are on `master`, so the check run won't show up until after merge, same deal as the existing translation-regression-comment workflow. Once merged, any PR touching Python code should get a "Python Unit Test Results" check with per-test annotations on failure.
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(ci): publish Python unit test results as PR check annotations [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #42503:
URL: https://github.com/apache/superset/pull/42503


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(ci): publish Python unit test results as PR check annotations [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #42503:
URL: https://github.com/apache/superset/pull/42503#discussion_r3707262808


##########
.github/workflows/superset-python-unittest-report.yml:
##########
@@ -0,0 +1,65 @@
+name: Python Unit Test Results
+
+on:
+  # zizmor: ignore[dangerous-triggers] - runs in base-branch context and only consumes artifacts uploaded by Python-Unit; never checks out PR code (see note below)
+  workflow_run:
+    workflows: ["Python-Unit"]
+    types: [completed]
+
+# This workflow publishes a check run annotating failing Python unit tests
+# inline on the PR diff, using JUnit XML uploaded by the Python-Unit workflow.
+# It uses the workflow_run trigger so that it always runs in the base-branch
+# context and can safely be granted write permissions, even for PRs from
+# forks or Dependabot.
+#
+# IMPORTANT: This workflow must NEVER check out code from the PR branch. All
+# data comes from artifacts uploaded by the Python-Unit workflow.
+permissions:
+  contents: read
+  checks: write
+  issues: read
+  actions: read
+
+jobs:
+  report:
+    runs-on: ubuntu-26.04
+    timeout-minutes: 10
+    if: >
+      github.event.workflow_run.conclusion == 'success' ||
+      github.event.workflow_run.conclusion == 'failure'
+    steps:
+      # Fails soft (continue-on-error) because the source unit-tests job is
+      # itself gated on change detection: a docs-only PR skips it entirely,
+      # so there is nothing to download or report on.
+      - name: Download JUnit results
+        id: download
+        continue-on-error: true
+        uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8
+        with:
+          pattern: "junit-results-*"
+          path: artifacts
+          merge-multiple: true

Review Comment:
   Good catch, thanks. On push builds both matrix legs run and their XMLs share filenames, so `merge-multiple: true` was letting one Python version clobber the other in `artifacts/`. Dropped `merge-multiple` in 399937a so each artifact download lands in its own subdirectory; the existing `artifacts/**/*.xml` glob is already recursive so it still picks up everything.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(ci): publish Python unit test results as PR check annotations [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #42503:
URL: https://github.com/apache/superset/pull/42503#issuecomment-5171107844

   ### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">🔨</span> Latest commit | 399937abd26df954adf353c81c631f77636ac6ee |
   |<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a70f30242880a0008f3ac8d |
   |<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-42503--superset-docs-preview.netlify.app](https://deploy-preview-42503--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTQyNTAzLS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.qrrs9OCs5lwplovcxduvWALpHa71yeVvsOKuaN6ZXY4)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   |<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcsxDoAgDADAv3SW2KhR4g98RhUIGoQGymCMf5ftlnuBc7pZYAXoYM8UD9_sLEnPdQ9n8Yof8Smqq8ZTlNgiKttSg5Q2jOWQns20M9OCbsRhGrRGQkTtRjq0ge8H) |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-42503--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(ci): publish Python unit test results as PR check annotations [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #42503:
URL: https://github.com/apache/superset/pull/42503#discussion_r3697368975


##########
.github/workflows/superset-python-unittest-report.yml:
##########
@@ -0,0 +1,65 @@
+name: Python Unit Test Results
+
+on:
+  # zizmor: ignore[dangerous-triggers] - runs in base-branch context and only consumes artifacts uploaded by Python-Unit; never checks out PR code (see note below)
+  workflow_run:
+    workflows: ["Python-Unit"]
+    types: [completed]
+
+# This workflow publishes a check run annotating failing Python unit tests
+# inline on the PR diff, using JUnit XML uploaded by the Python-Unit workflow.
+# It uses the workflow_run trigger so that it always runs in the base-branch
+# context and can safely be granted write permissions, even for PRs from
+# forks or Dependabot.
+#
+# IMPORTANT: This workflow must NEVER check out code from the PR branch. All
+# data comes from artifacts uploaded by the Python-Unit workflow.
+permissions:
+  contents: read
+  checks: write
+  issues: read
+  actions: read
+
+jobs:
+  report:
+    runs-on: ubuntu-26.04
+    timeout-minutes: 10
+    if: >
+      github.event.workflow_run.conclusion == 'success' ||
+      github.event.workflow_run.conclusion == 'failure'
+    steps:
+      # Fails soft (continue-on-error) because the source unit-tests job is
+      # itself gated on change detection: a docs-only PR skips it entirely,
+      # so there is nothing to download or report on.
+      - name: Download JUnit results
+        id: download
+        continue-on-error: true
+        uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8
+        with:
+          pattern: "junit-results-*"
+          path: artifacts
+          merge-multiple: true

Review Comment:
   On push runs both matrix artifacts contain these same XML filenames, and merging them into one directory lets one Python version overwrite the other, so this check can omit a failing `current` or `next` result. Could we keep the artifact-name directories (or include the Python version in each XML filename) before globbing them?



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(ci): publish Python unit test results as PR check annotations [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #42503:
URL: https://github.com/apache/superset/pull/42503#issuecomment-5100213052

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/42503?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 55.99%. Comparing base ([`3095d7b`](https://app.codecov.io/gh/apache/superset/commit/3095d7b07ffe431708b12c74d81c6e2e8742ab10?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`2f0e3bc`](https://app.codecov.io/gh/apache/superset/commit/2f0e3bce25197dbe1a6ddb9bb0c9e33893496b01?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   > :exclamation:  There is a different number of reports uploaded between BASE (3095d7b) and HEAD (2f0e3bc). Click for more details.
   > 
   > <details><summary>HEAD has 4 uploads less than BASE</summary>
   >
   >| Flag | BASE (3095d7b) | HEAD (2f0e3bc) |
   >|------|------|------|
   >|python|6|2|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #42503      +/-   ##
   ==========================================
   - Coverage   65.27%   55.99%   -9.29%     
   ==========================================
     Files        2795     2795              
     Lines      157832   157832              
     Branches    36079    36079              
   ==========================================
   - Hits       103022    88373   -14649     
   - Misses      52817    68630   +15813     
   + Partials     1993      829    -1164     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/42503/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/42503/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `38.30% <ø> (ø)` | |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/42503/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [postgres](https://app.codecov.io/gh/apache/superset/pull/42503/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/42503/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.22% <ø> (ø)` | |
   | [python](https://app.codecov.io/gh/apache/superset/pull/42503/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.27% <ø> (-18.75%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/42503/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/42503/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/42503?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(ci): publish Python unit test results as PR check annotations [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42503:
URL: https://github.com/apache/superset/pull/42503#issuecomment-5154610776

   <!-- Bito Reply -->
   The current implementation uses `merge-multiple: true` in `actions/download-artifact` to combine artifacts from different matrix runs into a single directory. Since the XML filenames are identical across Python versions, they overwrite each other. To resolve this, you can either keep the artifact-name directories (by setting `merge-multiple: false`) or include the Python version in the XML filenames during the `pytest` execution in `superset-python-unittest.yml`.
   
   **.github/workflows/superset-python-unittest-report.yml**
   ```
   with:
             pattern: "junit-results-*"
             path: artifacts
             merge-multiple: false
   ```
   
   **.github/workflows/superset-python-unittest.yml**
   ```
   run: |
               pytest ... --junit-xml=test-results/junit-unit-${{ matrix.python-version }}.xml
   ```


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(ci): publish Python unit test results as PR check annotations [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42503:
URL: https://github.com/apache/superset/pull/42503#issuecomment-5100186669

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org