ci: Upload coverage to GitHub Code Quality#1037
Open
Aaron ("AJ") Steers (aaronsteers) wants to merge 10 commits into
Open
ci: Upload coverage to GitHub Code Quality#1037Aaron ("AJ") Steers (aaronsteers) wants to merge 10 commits into
Aaron ("AJ") Steers (aaronsteers) wants to merge 10 commits into
Conversation
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This CDK VersionYou can test this version of the CDK using the following: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1779860596-github-coverage-upload#egg=airbyte-python-cdk[dev]' --help
# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1779860596-github-coverage-uploadPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
Contributor
Code Coverage OverviewLanguages: Python Python / code-coverage/pytest-fastThe overall coverage in the branch is 90%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
Python / code-coverage/pytest-matrixThe overall coverage in the branch is 90%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
Updated |
Copilot started reviewing on behalf of
Aaron ("AJ") Steers (aaronsteers)
May 27, 2026 06:40
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds GitHub Code Quality coverage uploads to the existing Python CDK pytest workflows while keeping existing coverage artifacts and test result publishing.
Changes:
- Adds
actions/upload-code-coverage@v1steps for fast pytest and representative matrix coverage XML. - Narrows generated Cobertura XML to
airbyte_cdk/*. - Adds explicit workflow/job permissions and includes the matrix workflow file in its path filters.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
.github/workflows/pytest_fast.yml |
Adds scoped permissions, filters coverage XML, and uploads fast-test coverage to GitHub Code Quality. |
.github/workflows/pytest_matrix.yml |
Adds scoped permissions, updates matrix path filters, filters coverage XML, and uploads representative matrix coverage to GitHub Code Quality. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
📝 WalkthroughWalkthroughTwo GitHub Actions workflows (pytest_fast and pytest_matrix) add workflow/job permissions, enable manual dispatch, scope coverage XML generation to ChangesGitHub Code Quality Coverage Reporting
🎯 3 (Moderate) | ⏱️ ~20 minutes Would you like me to suggest a brief consolidated snippet showing the 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/pytest_fast.yml (1)
51-52:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCheckout PR head SHA for accurate coverage mapping: On
pull_request, should we setactions/checkout@v4toref: ${{ github.event.pull_request.head.sha || github.sha }}so coverage maps to the PR head commit (not the synthetic merge commit)? (docs.github.com) wdyt?Suggested patch
- name: Checkout code uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha || github.sha }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pytest_fast.yml around lines 51 - 52, Update the checkout step to use the PR head SHA instead of the default merge commit by adding the with.ref setting to the actions/checkout@v4 step, specifically set ref: ${{ github.event.pull_request.head.sha || github.sha }} so coverage and other tooling run against the PR head commit (modify the "Checkout code" step that uses actions/checkout@v4 to include this with.ref).
🧹 Nitpick comments (1)
.github/workflows/pytest_matrix.yml (1)
59-60: ⚡ Quick winConsider checking out the PR head SHA for more accurate Code Quality coverage mapping
- For
pull_requestruns, does it make sense to check out${{ github.event.pull_request.head.sha || github.sha }}instead of the default merge/as-merged commit, so the coverage is mapped to the PR HEAD? wdyt?Suggested patch
- name: Checkout code uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha || github.sha }}
actions/upload-code-coverage@v1already uses the correctfile:input and the step hasif: always(), so the earlier concerns about the input key and upload being skipped on failures don’t apply.- Would you like to pin
actions/checkout@v4/actions/upload-code-coverage@v1to commit SHAs per your supply-chain policy?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pytest_matrix.yml around lines 59 - 60, Update the "Checkout code" step (uses: actions/checkout@v4) to checkout the PR head SHA for pull_request events by adding a with: ref: ${{ github.event.pull_request.head.sha || github.sha }} so coverage maps to the PR HEAD rather than the merge commit; optionally consider pinning the actions/checkout and actions/upload-code-coverage actions to specific commit SHAs per your supply-chain policy.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pytest_fast.yml:
- Line 99: Replace the floating reference uses: actions/upload-code-coverage@v1
with a pinned full commit SHA (e.g., uses:
actions/upload-code-coverage@<full-commit-sha>) so the action cannot change
unexpectedly; update this in both occurrences (the one in pytest_fast.yml and
the one in pytest_matrix.yml) by finding the desired commit SHA from the
actions/upload-code-coverage repository and substituting it for `@v1`, committing
the change.
In @.github/workflows/pytest_matrix.yml:
- Line 125: Replace the mutable tag "actions/upload-code-coverage@v1" with a
pinned immutable commit SHA: locate the desired release commit SHA from the
actions/upload-code-coverage repository (e.g., the latest stable commit), and
update the uses entry to "actions/upload-code-coverage@<COMMIT_SHA>" so the
workflow references that exact commit; ensure you commit the change and run the
workflow to verify behavior remains the same.
---
Outside diff comments:
In @.github/workflows/pytest_fast.yml:
- Around line 51-52: Update the checkout step to use the PR head SHA instead of
the default merge commit by adding the with.ref setting to the
actions/checkout@v4 step, specifically set ref: ${{
github.event.pull_request.head.sha || github.sha }} so coverage and other
tooling run against the PR head commit (modify the "Checkout code" step that
uses actions/checkout@v4 to include this with.ref).
---
Nitpick comments:
In @.github/workflows/pytest_matrix.yml:
- Around line 59-60: Update the "Checkout code" step (uses: actions/checkout@v4)
to checkout the PR head SHA for pull_request events by adding a with: ref: ${{
github.event.pull_request.head.sha || github.sha }} so coverage maps to the PR
HEAD rather than the merge commit; optionally consider pinning the
actions/checkout and actions/upload-code-coverage actions to specific commit
SHAs per your supply-chain policy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a8f887c6-d74a-4488-9a2b-dda0645defc8
📒 Files selected for processing (2)
.github/workflows/pytest_fast.yml.github/workflows/pytest_matrix.yml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pytest_fast.yml:
- Around line 54-55: Update the actions/checkout@v4 step to explicitly disable
credential persistence by adding the input key persist-credentials: false
alongside the existing with.ref setting; locate the checkout step
(actions/checkout@v4) and add persist-credentials: false under its with block so
the runtime token is not written to local git config.
In @.github/workflows/pytest_matrix.yml:
- Around line 63-64: Set the checkout action to avoid persisting the runner
token by adding persist-credentials: false to the actions/checkout@v4 step;
locate the checkout step referencing "actions/checkout@v4" and update its with:
block to include the key "persist-credentials" with value false so the token is
not left in git config after checkout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8d723ef5-1c64-4c3b-9760-adb2d60f042c
📒 Files selected for processing (2)
.github/workflows/pytest_fast.yml.github/workflows/pytest_matrix.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
actions/upload-code-coverage@v1uploads for the Python CDK's existing Cobertura XML reports from Pytest Fast and the representative full pytest matrix job.code-quality: writeto only the pytest jobs that upload coverage while preservingchecks: writefor jobs that publish test result check runs.airbyte_cdk/*package files so GitHub Code Quality does not report stdlib or test-helper paths.workflow_dispatchto both coverage-producing pytest workflows so maintainers can manually refresh coverage frommainor a PR branch.htmlcov/coverage.xmlexisting so failed/skipped coverage generation does not produce noisy upload errors.fail-on-error: false.Review & Testing Checklist for Human
code-coverage/pytest-fastandcode-coverage/pytest-matrix, are useful and not too noisy in the PR coverage UI.mainupload coverage.mainafter amaincoverage upload is available.Publish Test Resultscheck-run summaries still appear after the explicit permissions change.Notes
checks: write, gate uploads on coverage XML, add manual dispatch, ensure manual dispatch bypasses path-filter skips, and disable checkout credential persistence.yq e '.' .github/workflows/pytest_fast.yml,yq e '.' .github/workflows/pytest_matrix.yml,yq e '.on.workflow_dispatch'for both workflows, andgit diff --check.Devin session
Requested by: Aaron ("AJ") Steers (@aaronsteers)