USHIFT-7409: CI Doctor: list all jobs - #235
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pmtk The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughChangesPeriodic job collection now persists per-release status snapshots, and the HTML report loads those snapshots to display pass rates, expandable job tables, failure issue links, and side-by-side Periodics layouts. Periodic status reporting
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
plugins/shared/scripts/create-report.py (2)
1437-1441: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPass-rate calculation (
passed/total/rate) is duplicated in four places.The same
passed = sum(1 for j in status if j.get("status") == "success")/rate = round(passed/total*100)logic is recomputed independently inrender_release_section(lines 1440-1441), the overview cards (lines 1748-1750), the TOC (lines 1799-1800), and the console summary (lines 2108-2109). As per path instructions ("avoid derived-state/sync drift (single source of truth)" from CONTRIBUTING.md), extracting a single helper (e.g.,_pass_rate(status) -> (passed, total, rate)) and reusing it everywhere would prevent these four call sites from silently drifting if the "success" semantics ever change.♻️ Proposed helper
def _pass_rate(status): total = len(status) if status else 0 passed = sum(1 for j in status if j.get("status") == "success") if status else 0 rate = round(passed / total * 100) if total > 0 else 0 return passed, total, rateAlso applies to: 1746-1751, 1797-1801, 2101-2111
🤖 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 `@plugins/shared/scripts/create-report.py` around lines 1437 - 1441, Extract the duplicated pass-rate calculation into a shared _pass_rate(status) helper returning passed, total, and rate, handling empty or None status safely. Replace the independent calculations in render_release_section, the overview cards, the TOC, and the console summary with calls to this helper so all success semantics remain centralized.Source: Path instructions
1699-1729: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNew parsing helpers lack test coverage.
_format_epoch,_format_duration, and_build_job_issue_mapare new parsing/mapping logic with several edge cases (missing/invalid values, epoch formatting, empty maps) but no accompanying tests were included in this PR's file set. As per path instructions, "Ensure any new parsing/validation logic has positive+negative tests."Want me to draft unit tests for these three functions (valid input, missing/None input, malformed input)?
🤖 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 `@plugins/shared/scripts/create-report.py` around lines 1699 - 1729, The new helpers _format_epoch, _format_duration, and _build_job_issue_map need unit-test coverage. Add positive and negative tests covering valid values, missing or None inputs, malformed values, empty releases/issues, and affected jobs, including expected formatting and mapping results; follow the repository’s existing test conventions.Source: Path instructions
🤖 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 `@plugins/shared/scripts/create-report.py`:
- Around line 1699-1704: Update _format_epoch to parse numeric epoch strings
through float before converting to int, matching _format_duration’s handling of
fractional values while preserving the existing UTC formatting and fallback
behavior.
- Around line 1437-1488: Update the sorted_status ordering in the release_status
section to normalize every job’s finished value to a numeric sort key, using
zero for missing or falsy values. Preserve the existing finished-based ordering
while preventing comparisons between numeric timestamps and strings.
---
Nitpick comments:
In `@plugins/shared/scripts/create-report.py`:
- Around line 1437-1441: Extract the duplicated pass-rate calculation into a
shared _pass_rate(status) helper returning passed, total, and rate, handling
empty or None status safely. Replace the independent calculations in
render_release_section, the overview cards, the TOC, and the console summary
with calls to this helper so all success semantics remain centralized.
- Around line 1699-1729: The new helpers _format_epoch, _format_duration, and
_build_job_issue_map need unit-test coverage. Add positive and negative tests
covering valid values, missing or None inputs, malformed values, empty
releases/issues, and affected jobs, including expected formatting and mapping
results; follow the repository’s existing test conventions.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 3f92d3bc-1a7c-46d7-977d-2c7db715668b
📒 Files selected for processing (2)
plugins/shared/scripts/create-report.pyplugins/shared/scripts/doctor.sh
|
/lgtm |
Adds list of all jobs (both successful and failed) for each release. They can also be viewed side by side.



Summary by CodeRabbit