fix: remove unit_tests directory from coverage report and add handling of tmp files#557
Conversation
Christo Grabowski (ChristoGrab)
requested a review
from Aaron ("AJ") Steers (aaronsteers)
📝 WalkthroughWalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant CoverageTool
participant SourceFiles
participant ExcludedFiles
CoverageTool->>SourceFiles: Analyze coverage for files in ./ and /tmp/
CoverageTool->>ExcludedFiles: Omit /tmp/tmp*.py and unit_tests/*
Note right of CoverageTool: Aggregate coverage using mapped paths
Would you like to see a more detailed breakdown of how the Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.coveragerc (1)
17-21: Could you clarify the/tmp/entry in the[paths]section?
I saw a previous question about why we include/tmp/as a source path. Is this mapping strictly needed for coverage of files generated in/tmp/, or could we scope it more narrowly? wdyt?
🧹 Nitpick comments (1)
.coveragerc (1)
12-16: Could we refine the omit patterns for test and temp files?
The patterns/tmp/tmp*.pyandunit_tests/*will only catch top-level files in those directories. If we have nested files underunit_tests/, they won’t be excluded. Would you consider switching to something likeunit_tests/**(or*/unit_tests/**) to recursively omit everything? Also, isunit_teststhe exact test directory name in our repo, or should we also exclude other paths liketests/? wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
.coveragerc(1 hunks)
Christo Grabowski (ChristoGrab)
deleted the
christo/fix-pytest-coverage-report
branch
What
Trying to address CI failures in #556, where Pytest steps are failing on the coverage report step despite the test suites passing.
Link to failure: https://github.com/airbytehq/airbyte-python-cdk/actions/runs/15118605619/job/42495441682?pr=556
This PR fixes CI coverage report errors by updating the
.coveragercconfiguration to properly handle temporary files and establish correct path mappings. Also excludes test files from the coverage report.Summary by CodeRabbit
Important
Auto-merge enabled.
This PR is set to merge automatically when all requirements are met.