chore: modernize CI and lint config#496
Conversation
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThe PR updates uv-based CI execution, project metadata and lint configuration, README setup instructions, helper implementations, browser test setup, and several test definitions and formatting details. ChangesTooling and test updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Allure Report Summary
|
95be7c7 to
b8664f6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utilities/vrt_helper.py (1)
89-98: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winMissing second argument in
check.equal.The
check.equalassertion here is missing its second argument (TestRunStatus.OK.name), which will cause aTypeErrorwhen this method is executed. Compare this with the correct usage in theshoot_pagemethod.🐛 Proposed fix
check.equal( self.vrt_tracker.track( TestRun( name=baseline_name, diffTollerancePercent=Constants.DIFF_TOLERANCE_PERCENT, imageBase64=self.driver.get_screenshot_as_base64(), ignoreAreas=ignore_areas, ) ).testRunResponse.status.name, + TestRunStatus.OK.name, )🤖 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 `@src/utilities/vrt_helper.py` around lines 89 - 98, Update the check.equal call in the VRT tracking flow to pass TestRunStatus.OK.name as its expected-value second argument, matching the established usage in shoot_page and preserving the current tracked status as the actual value.
🧹 Nitpick comments (2)
src/utilities/vrt_helper.py (1)
60-62: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCorrect typo in method name.
The method name contains a typo:
anginstead ofand.♻️ Proposed refactor
- def shoot_page_ang_ignore_elements( + def shoot_page_and_ignore_elements( self, baseline_name: str, elements: list[WebElement] ) -> None:🤖 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 `@src/utilities/vrt_helper.py` around lines 60 - 62, Rename the VRT helper method shoot_page_ang_ignore_elements to shoot_page_and_ignore_elements, correcting ang to and, and update all call sites and references to use the new name.src/pages/base_page.py (1)
101-102: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd missing semicolon and handle
Nonefororiginal_style.When concatenating the
original_style, there is no semicolon separating it from the new border style, which can result in invalid CSS (e.g.,1px solid greencolor: red;). Additionally, if the element has no style attribute,get_attribute("style")might returnNone, resulting in the string"None"being appended.Consider handling
Noneand ensuring proper CSS separation.♻️ Proposed refactor
- original_style = webelement.get_attribute("style") - new_style = f"background-color:yellow;border: 1px solid {color}{original_style}" + original_style = webelement.get_attribute("style") or "" + new_style = f"background-color:yellow;border: 1px solid {color}; {original_style}"🤖 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 `@src/pages/base_page.py` around lines 101 - 102, Update the style construction near original_style and new_style to treat a missing style attribute as an empty string and insert a semicolon between the new border declaration and the preserved original styles. Keep the existing background and border values unchanged while ensuring the resulting CSS never appends the literal "None".
🤖 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/pr.yml:
- Around line 43-46: Update the pytest step in the workflow to pass BASE_URL
through the step’s env block, alongside EMAIL and PASSWORD, then reference the
environment variable in the run command instead of expanding vars.BASE_URL
directly. Preserve the existing pytest options and base URL behavior.
---
Outside diff comments:
In `@src/utilities/vrt_helper.py`:
- Around line 89-98: Update the check.equal call in the VRT tracking flow to
pass TestRunStatus.OK.name as its expected-value second argument, matching the
established usage in shoot_page and preserving the current tracked status as the
actual value.
---
Nitpick comments:
In `@src/pages/base_page.py`:
- Around line 101-102: Update the style construction near original_style and
new_style to treat a missing style attribute as an empty string and insert a
semicolon between the new border declaration and the preserved original styles.
Keep the existing background and border values unchanged while ensuring the
resulting CSS never appends the literal "None".
In `@src/utilities/vrt_helper.py`:
- Around line 60-62: Rename the VRT helper method shoot_page_ang_ignore_elements
to shoot_page_and_ignore_elements, correcting ang to and, and update all call
sites and references to use the new name.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fff6c796-ce7b-4ef0-8223-45fa0ad0fdc5
📒 Files selected for processing (14)
.github/workflows/monthly.yml.github/workflows/pr.ymlREADME.mdpyproject.tomlsrc/pages/base_page.pysrc/utilities/excel_parser.pysrc/utilities/mailinator_helper.pysrc/utilities/vrt_helper.pytests/conftest.pytests/dependency_class_test.pytests/dependency_test.pytests/email_test.pytests/login_test.pytests/workspaces_test.py
b8664f6 to
1083dcf
Compare
1083dcf to
3dca22a
Compare
3dca22a to
dfc0f08
Compare
dfc0f08 to
02628be
Compare
02628be to
026f2f9
Compare
Summary
Verification
Summary by CodeRabbit
Documentation
Bug Fixes
@characters.Tests