NO-JIRA: fix(ci): build Playwright browser-tests image on rhoai-3.4 - #2744
Conversation
Backport the browser-tests-image step from opendatahub-io#4096 so codeserver Playwright runs pass the required playwright-image input. Without it PLAYWRIGHT_IMAGE is empty and podman inspect fails with a Go JSON unmarshalling error. Co-authored-by: Cursor <cursoragent@cursor.com>
CI status [antigravity]Run: Build Notebooks (push) #31591543046 — 3/3 complete · 3 skipped No workbench image jobs ran; all matrix jobs were skipped. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe workflow builds a dedicated Playwright image for successful codeserver builds. Playwright tests use the generated image and upload a platform- and build-type-specific JUnit artifact. ChangesPlaywright workflow
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
CI status [antigravity]Run: Build Notebooks (pr) #31591554770 — 3/3 complete · 1 passed · 2 skipped No workbench image jobs ran; all matrix jobs were skipped. |
📋 Review SummaryThis PR cleanly backports the 🔍 General Feedback
I did not post inline review comments. |
jiridanek
left a comment
There was a problem hiding this comment.
@coderabbitai review summary
Overview: Faithful backport of the Build browser tests image step from rhoai-3.5/upstream opendatahub-io/notebooks#4096 into rhoai-3.4's build-notebooks-TEMPLATE.yaml, plus wiring playwright-image (now required: true on the composite action since #2545) and junit-artifact-file into the playwright-test action call.
Verification performed
- Diffed the new block line-by-line against the equivalent section in
rhoai-3.5'sbuild-notebooks-TEMPLATE.yaml— byte-identical aside from extra explanatory comments justifying the backport. - Confirmed
scripts/get_playwright_version.pyandtests/browser/Dockerfileexist on this branch. - Confirmed
.github/actions/playwright-test/action.ymlrequiresplaywright-imageand consumesjunit-artifact-file— both are now supplied, matching the describedpodman inspect ''failure mode this PR fixes. - Checked for
id:collisions in the file —browser-tests-imageis unique. - CI:
Test Template with Minimal Notebook / rhoaipassed (12m26s); no failing checks.tideis blocked only on theapprovedlabel (process gate, not a code issue).
Verdict: Clean, minimal, correct backport. No correctness, security, or convention issues found. See inline notes below for confirmation points (informational only, no changes requested).
| # Backport of opendatahub-io/notebooks#4096: playwright-test requires playwright-image; | ||
| # without this step PLAYWRIGHT_IMAGE is empty and `podman inspect ''` fails with | ||
| # "json: cannot unmarshal array into Go value of type define.InspectContainerData". | ||
| - name: Build browser tests image |
There was a problem hiding this comment.
Confirmed: this step's build invocation (podman build --build-arg PLAYWRIGHT_VERSION=... -t localhost/workbench-images-tests:ci -f tests/browser/Dockerfile tests/browser) matches build-browser-tests.yaml and the equivalent step in rhoai-3.5's template byte-for-byte. scripts/get_playwright_version.py and tests/browser/Dockerfile both exist on this branch.
| echo "image=${IMAGE}" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Run Playwright tests | ||
| if: ${{ !cancelled() && contains(inputs.target, 'codeserver') && steps.make-target.outcome == 'success' && steps.browser-tests-image.outcome == 'success' }} |
There was a problem hiding this comment.
Good belt-and-suspenders guard: gating on steps.browser-tests-image.outcome == 'success' in addition to the existing contains(inputs.target, 'codeserver') check ensures Run Playwright tests never executes with an empty playwright-image — which was the root cause of the podman inspect '' failure this PR fixes.
| uses: ./.github/actions/playwright-test | ||
| with: | ||
| test-target: ${{ steps.calculated_vars.outputs.OUTPUT_IMAGE }} | ||
| playwright-image: ${{ steps.browser-tests-image.outputs.image }} |
There was a problem hiding this comment.
Verified: playwright-image is required: true on .github/actions/playwright-test/action.yml since #2545, so this input was indeed missing before this fix. No other callers of this composite action in the repo were left unpatched (checked).
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ide-developer 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 |
Summary
Build browser tests imagestep from opendatahub-io/notebooks#4096 /rhoai-3.5intorhoai-3.4'sbuild-notebooks-TEMPLATE.yaml.playwright-image(andjunit-artifact-file) into the composite action that already requires it after PR #2545.PLAYWRIGHT_IMAGE→podman inspect ''→json: cannot unmarshal array into Go value of type define.InspectContainerData.Test plan
localhost/workbench-images-tests:cibefore Playwrightcodeserver)Made with Cursor
Summary by CodeRabbit