odh-dashboard: add required Prow lint, type-check, unit-tests - #83263
odh-dashboard: add required Prow lint, type-check, unit-tests#83263manaswinidas wants to merge 8 commits into
Conversation
|
@manaswinidas: GitHub didn't allow me to request PR reviews from the following users: opendatahub-io/dashboard-maintainers. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. WalkthroughThe CI configuration adds a Node.js 22 source container, resource requests, and jobs for dependency installation, linting, type-checking, and unit testing. ChangesODH Dashboard CI validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change adds required Prow lint, type-check, and unit-test gates without any identified current-head merge-blocking risk; it is merge-ready after normal checks and review. Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.) Full details: Stable And Deterministic Test NamesExplanation PASS. The PR changes only CI configuration and generated presubmit YAML; it does not add or modify Ginkgo test source or any It/Describe/Context/When title. The added CI identifiers ( Full details: Test Structure And QualityExplanation PASS — the complete PR changes only two CI YAML files: the odh-dashboard ci-operator configuration and generated presubmit configuration. The added checks run npm lint, type-check, and unit-test commands. The patch adds no Ginkgo test code, cluster operations, Eventually/Consistently calls, or Ginkgo assertions. Therefore, the stated Ginkgo test-quality requirements are not applicable. Full details: Microshift Test CompatibilityExplanation PASS: The pull request changes only ci-operator configuration and generated presubmit jobs. The diff adds a Node.js image and npm lint, type-check, and unit-test commands; it adds no Ginkgo e2e tests, OpenShift API references, MicroShift assumptions, or test labels requiring compatibility review. The MicroShift test check is therefore not applicable. Full details: Single Node Openshift (Sno) Test CompatibilityExplanation PASS — the pull request changes only CI configuration files: the ci-operator YAML and generated presubmit jobs. The committed diff from base 97027ea contains no Go files and adds no Ginkgo declarations such as It(), Describe(), Context(), or When(). The new lint, type-check, and unit-tests entries are Node.js container jobs, so the SNO-specific Ginkgo e2e compatibility check is not applicable. Full details: Topology-Aware Scheduling CompatibilityExplanation PASS — the pull request changes only Full details: Ote Binary Stdout ContractExplanation PASS — The pull request changes only ci-operator YAML configuration and generated Prow job YAML. The new jobs run shell commands and npm scripts in a Node.js image; they do not add or modify an OpenShift Tests Extension binary, Go suite setup, or process-level OTE code. The changed lines contain no OTE markers or stdout-writing code, so the stated JSON-on-stdout contract is not applicable. Full details: Ipv6 And Disconnected Network Test CompatibilityExplanation PASS: The pull request changes only CI YAML configuration. It adds lint, type-check, and unit-test jobs, but no Ginkgo e2e test declarations or test source files. The IPv6 and disconnected-network Ginkgo check is therefore not applicable. Full details: No-Weak-CryptoExplanation PASS. The PR changes only the odh-dashboard ci-operator config and generated presubmit YAML. Added content defines a Node.js image, npm quality-test commands, temporary workspaces, and Prow job metadata. The PR-added lines contain no MD5, SHA1, DES, 3DES, RC4, Blowfish, or ECB usage, crypto APIs, custom crypto implementation, or non-constant-time secret/token comparison. Secret references are CI credential mounts only. Full details: Container-PrivilegesExplanation No explicit privilege violation was introduced. The only root reference is the image-build step Full details: No-Sensitive-Data-In-LogsExplanation No sensitive-data logging was introduced. The pull request adds build and test commands only: ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
needs-ok-to-test
|
Hi @manaswinidas. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: manaswinidas The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@ci-operator/config/opendatahub-io/odh-dashboard/opendatahub-io-odh-dashboard-main.yaml`:
- Around line 42-49: Update the nodejs-22-src build definition by removing the
from: src override so it uses the Dockerfile’s Node.js 22 base image, and change
COPY --from=src . . to copy directly from the repository build context.
🪄 Autofix
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: Pro Plus
Run ID: 56b8377b-5830-48f1-90ae-698c9bb4cffd
⛔ Files ignored due to path filters (1)
ci-operator/jobs/opendatahub-io/odh-dashboard/opendatahub-io-odh-dashboard-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (1)
ci-operator/config/opendatahub-io/odh-dashboard/opendatahub-io-odh-dashboard-main.yaml
|
/ok-to-test |
|
@manaswinidas: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test @christianvogt please take over here -- let me know if you need me. |
openshift-ci
Bot
added
ok-to-test
AI Review
|
|
@christianvogt Addressed your review in 0562536:
Please take another look when you can. |
|
/pj-rehearse pull-ci-opendatahub-io-odh-dashboard-main-lint pull-ci-opendatahub-io-odh-dashboard-main-type-check pull-ci-opendatahub-io-odh-dashboard-main-unit-tests |
|
@christianvogt: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-ci
Bot
added
the
needs-rebase
|
@manaswinidas, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
fb1282f to
afaa4b0
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
openshift-ci
Bot
removed
the
needs-rebase
|
/pj-rehearse pull-ci-opendatahub-io-odh-dashboard-main-lint pull-ci-opendatahub-io-odh-dashboard-main-type-check pull-ci-opendatahub-io-odh-dashboard-main-unit-tests |
|
@manaswinidas: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
This still necessary? probably need to rebase the branch @manaswinidas |
|
Yes, this is important and Christian is reviewing this |
|
@manaswinidas the jobs are still failing and will need to be fixed I also started a thread with @antowaddle to get his thoughts on this solution. But feel free to comment here too |
|
Pushed 9cc036c: copy the tree into /pj-rehearse pull-ci-opendatahub-io-odh-dashboard-main-lint pull-ci-opendatahub-io-odh-dashboard-main-type-check pull-ci-opendatahub-io-odh-dashboard-main-unit-tests |
|
@manaswinidas: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
EPERM is fixed. The remaining failures were not product lint/type/unit bugs — That is why:
GHA clones git, so it never hits this. Pushed 67ef92c: drop source /pj-rehearse pull-ci-opendatahub-io-odh-dashboard-main-lint pull-ci-opendatahub-io-odh-dashboard-main-type-check pull-ci-opendatahub-io-odh-dashboard-main-unit-tests |
|
@manaswinidas: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-opendatahub-io-odh-dashboard-main-unit-tests |
|
@manaswinidas: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
Pushed cac679c: raise Jest timeout to 15s for unit-tests ( /pj-rehearse pull-ci-opendatahub-io-odh-dashboard-main-unit-tests |
|
@manaswinidas: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
cac679c to
e46036c
Compare
Tide only retests Prow contexts before merge. Quality gates lived only in GitHub Actions, so green PRs could still race and break main (RHOAIENG-3333). Add a Node 22 test image (nodejs-22-src) while keeping the golang build_root for product image builds, and required always-run container tests for lint, type-check, and unit-tests. Quality jobs use skip_cloning so ci-operator gets a full checkout instead of the Dockerfile-only sparse checkout used by image jobs. Co-authored-by: Cursor <cursoragent@cursor.com>
Drop from: src so the test image keeps the Node 22 base and COPY the repo context. Match odh-dashboard GHA install (npm@11.8.0 + install:modules). Regenerate presubmits with make update instead of hand-editing skip_cloning. Co-authored-by: Cursor <cursoragent@cursor.com>
COPY as root left source owned by root; USER 1001 then failed chmod on package-subtree.sh during npm ci (pj-rehearse EPERM). Co-authored-by: Cursor <cursoragent@cursor.com>
Rebase onto main picked up odh-mod-arch-notebooks. prowgen lists that Dockerfile on every job; lint/type-check/unit-tests were generated before it existed. Co-authored-by: Cursor <cursoragent@cursor.com>
OpenShift CI pods run as a random UID, so npm ci chmod on image-layer files owned by 1001 fails with EPERM. Copy the tree into mktemp so the test UID owns it. --no-bin-links is not viable because these jobs need package binaries. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: manaswinidas <dasmanaswini10@gmail.com>
COPY . . applies odh-dashboard .dockerignore, which drops **/__tests__/ and **/__mocks__/. That made lint see an empty autox-core ui/src, type-check miss mock modules, and unit tests skip real suites. clone: true uses the src git tree instead, so tests match GitHub Actions and the test UID owns the files. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: manaswinidas <dasmanaswini10@gmail.com>
PipelineRunTable compare-runs test flakes on Prow under load (5000ms default). Pass --testTimeout=15000 through turbo. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: manaswinidas <dasmanaswini10@gmail.com>
GHA runs test-unit-coverage, which skips packages such as maas that only define test-unit. Prow was running test-unit and failing on maas duplicate-React errors in OverviewTable.spec.tsx. Co-authored-by: Cursor <cursoragent@cursor.com>
e46036c to
962dd6d
Compare
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
1 similar comment
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@manaswinidas: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
4 participants
Summary
Addresses RHOAIENG-3333: Tide only retests Prow contexts against the latest base/merge SHA. odh-dashboard quality gates (lint / type-check / unit) currently run only in GitHub Actions, so two green PRs can still race and break
main(recent example: type-check break from mock import move vs new tests).Changes
build_rootand product image builds/mirrorsnodejs-22-src(UBI Node 22 + source viaCOPY --from=src) for quality tests; exclude it from promotionlint—npm ci+NODE_OPTIONS=--max-old-space-size=8192 npm run linttype-check—npm ci+npm run type-checkunit-tests—npm ci+npm run test-unitskip_cloning: trueso ci-operator gets a full checkout (image jobs keep Dockerfile sparse checkout)Phase 1 only. Cypress / contract tests stay on GitHub Actions for now (dual-run).
Test plan
/pj-rehearse pull-ci-opendatahub-io-odh-dashboard-main-lintagainst an odh-dashboard PR/pj-rehearse pull-ci-opendatahub-io-odh-dashboard-main-type-check/pj-rehearse pull-ci-opendatahub-io-odh-dashboard-main-unit-testsci/prow/lint,ci/prow/type-check,ci/prow/unit-testsare required (nooptional: true)/cc @opendatahub-io/dashboard-maintainers
Made with Cursor
Summary by CodeRabbit
Updates the
odh-dashboardOpenShift CI configuration to add Prow quality gates. The configuration now:nodejs-22-srctest image.lint,type-check, andunit-testscontainer tests.HOMEfor random-UID CI execution.