chore: workflow fixes and devcontainer realign for playwright, initia…#1792
Merged
Conversation
…l demo UI test docs
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughUpdates Playwright setup in the devcontainer and MCP, adds Playwright verification tooling and UI test docs, and refactors CI into parallel build/modern-tests/legacy-tests with explicit Docker buildx caching and container lifecycle management. Changes
Sequence Diagram(s)sequenceDiagram
participant Actions as "GitHub Actions"
participant Buildx as "Docker Buildx\n(build devcontainer image)"
participant Registry as "Image Cache/Registry"
participant Modern as "Modern Tests\n(JUnit5 + H2)"
participant Legacy as "Legacy Tests\n(JUnit4 + MariaDB)"
participant DB as "MariaDB Container"
participant App as "Dev Tomcat Container / App"
rect rgba(200,230,255,0.5)
Actions->>Buildx: build devcontainer image (cached)
Buildx->>Registry: push/populate cache
end
rect rgba(200,255,200,0.5)
Actions->>Modern: start modern-tests job
Modern->>App: start devcontainer (no DB)
Modern->>Modern: run JUnit5 tests (H2)
Modern-->>Actions: upload artifacts/results
end
rect rgba(255,230,200,0.5)
Actions->>Legacy: start legacy-tests job
Legacy->>App: start devcontainer
Legacy->>DB: start MariaDB container on network
DB->>Legacy: respond to readiness probe
Legacy->>App: initialize providers/databases
Legacy->>Legacy: run JUnit4 tests (against MariaDB)
Legacy-->>Actions: upload artifacts/results
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. OpenSSF Scorecard
Scanned Files
|
There was a problem hiding this comment.
Pull request overview
This pull request updates the CI/CD workflow structure, aligns DevContainer configuration with Playwright 1.56.0, and introduces comprehensive UI test documentation for the Demographic module. The changes modernize the testing infrastructure by introducing parallel test execution and updating Playwright dependencies.
Changes:
- Restructured GitHub Actions workflow to run modern and legacy tests in parallel after a shared build step
- Updated Playwright from 1.40.0 to 1.56.0 in the DevContainer with proper Ubuntu 24.04 t64 package support
- Added comprehensive Test 2 documentation covering 30-step demographic module validation workflow
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
.mcp.json |
Updates Playwright MCP version to 0.0.56 and adjusts Chromium executable path (contains version mismatch issue) |
.devcontainer/development/Dockerfile |
Upgrades Playwright to 1.56.0 with --with-deps flag for proper Ubuntu 24.04 t64 library installation |
.github/workflows/maven-project.yml |
Restructures workflow into parallel build→test jobs with BuildKit caching and separate modern/legacy test execution |
docs/ui-tests/test-2/test-2-README.md |
Adds comprehensive documentation for 30-step demographic module UI test including prerequisites, workflow phases, and troubleshooting |
docs/ui-tests/test-2/test-2-EXECUTION.md |
Provides detailed step-by-step execution guide with expected screenshots, database verification, and success criteria |
docs/ui-tests/test-2/screenshots/.gitkeep |
Creates directory structure for test screenshots |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Co-authored-by: yingbull <8680161+yingbull@users.noreply.github.com>
Co-authored-by: yingbull <8680161+yingbull@users.noreply.github.com>
Co-authored-by: yingbull <8680161+yingbull@users.noreply.github.com>
Co-authored-by: yingbull <8680161+yingbull@users.noreply.github.com>
Fix MCP package version to use @latest per official documentation
Co-authored-by: yingbull <8680161+yingbull@users.noreply.github.com>
Use artifacts to share Maven dependencies between workflow jobs
…ation Co-authored-by: yingbull <8680161+yingbull@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@docs/ui-tests/test-2/test-2-EXECUTION.md`:
- Line 69: Wrap bare URLs/domains in code or link syntax to satisfy MD034:
locate the bare URL "http://localhost:8080/oscar" (and the other occurrences
noted at lines 177 and 353) in docs/ui-tests/test-2/test-2-EXECUTION.md and
replace each bare URL with either a code span (e.g.,
`http://localhost:8080/oscar`) or a proper markdown link (e.g.,
[http://localhost:8080/oscar](http://localhost:8080/oscar)).
- Around line 32-40: The expected-output code fence in the test markdown is
missing a language tag causing MD040; edit the fenced block named "Expected
output" and change the opening triple-backtick from ``` to ```text so the block
becomes a text-specified fenced code block (keep the table contents unchanged)
to satisfy the linter and improve readability.
In `@docs/ui-tests/test-2/test-2-README.md`:
- Around line 34-47: The markdown tables under the headings "Provider
Credentials" and "Existing Test Patients (Must Exist)" need blank lines before
and after them to satisfy MD058; edit the README to insert one empty line above
each table and one empty line below each table so each table is separated from
surrounding headings and paragraphs.
- Around line 138-163: The fenced code blocks under the headings "Last Name:
TEST-UITEST2" (patient data block), "Edit Values (Applied During Test)" and
"Cleanup Values" need a language label to satisfy MD040 and improve readability;
update each triple-backtick fence to include a language like text (e.g.,
```text) so the blocks for the patient data, edit values, and cleanup values are
explicitly marked as plain text.
- Line 28: Replace the bare URL "http://localhost:8080/oscar" in the sentence
starting with "OpenO EMR running at" (and the other occurrences at the same
lines referenced) with either a code span or a proper Markdown link; e.g.,
change the inline bare domain to `http://localhost:8080/oscar` or
`[http://localhost:8080/oscar](http://localhost:8080/oscar)` so the markdown
linter no longer flags the bare URL.
♻️ Duplicate comments (1)
.mcp.json (1)
6-13: Verify MCP version and Chromium path align with Playwright 1.56.0.Please confirm
@playwright/mcp@0.0.56is compatible with Playwright 1.56.0 and that the configured Chromium path exists after the devcontainer install; otherwise MCP won’t be able to launch the browser.🔍 Suggested verification commands
# Check actual Playwright browser install paths ls -d /root/.cache/ms-playwright/chromium-*/chrome-linux*/chrome # Confirm which Chromium would be installed by Playwright 1.56.0 npx -y playwright@1.56.0 install chromium --dry-run
🧹 Nitpick comments (2)
.devcontainer/development/Dockerfile (1)
66-75: Pin Playwright browser install to the same version as the global CLI.
npx playwright installcan resolve a newer Playwright package if no local install exists. Safer to use the global CLI (or an explicitly versionednpx) to keep browser binaries aligned with 1.56.0.♻️ Suggested change
-RUN npx playwright install --with-deps chromium chrome firefox && \ +RUN playwright install --with-deps chromium chrome firefox && \ echo "✅ Playwright browsers installed (Chromium, Chrome, Firefox)"docs/ui-tests/test-2/test-2-README.md (1)
137-149: Use FAKE- prefix for synthetic patient names in UI-test docs.Consider renaming
TEST-UITEST2to a FAKE-prefixed value (e.g.,FAKE-UITEST2) and updating references in this README and the execution guide to keep synthetic data conventions consistent across OpenO EMR UI-test docs. Based on learnings, please keep example data explicitly synthetic.
Comment on lines
+32
to
+40
| Expected output: | ||
| ``` | ||
| +----------------+--------------+-------------+----------------+ | ||
| | demographic_no | last_name | first_name | patient_status | | ||
| +----------------+--------------+-------------+----------------+ | ||
| | 1 | FAKE-Jones | FAKE-Jacky | AC | | ||
| | 182 | FAKE-Gaylord | FAKE-Branda | AC | | ||
| +----------------+--------------+-------------+----------------+ | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the expected-output fence.
This resolves the MD040 lint warning and improves readability.
✏️ Suggested change
-Expected output:
-```
+Expected output:
+```text
+----------------+--------------+-------------+----------------+
| demographic_no | last_name | first_name | patient_status |
+----------------+--------------+-------------+----------------+
| 1 | FAKE-Jones | FAKE-Jacky | AC |
| 182 | FAKE-Gaylord | FAKE-Branda | AC |
+----------------+--------------+-------------+----------------+
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/ui-tests/test-2/test-2-EXECUTION.md` around lines 32 - 40, The
expected-output code fence in the test markdown is missing a language tag
causing MD040; edit the fenced block named "Expected output" and change the
opening triple-backtick from ``` to ```text so the block becomes a
text-specified fenced code block (keep the table contents unchanged) to satisfy
the linter and improve readability.
| ## Prerequisites | ||
|
|
||
| ### Application Requirements | ||
| - OpenO EMR running at http://localhost:8080/oscar |
There was a problem hiding this comment.
Avoid bare URLs/domains in prose and tables.
Markdownlint flags these as bare URLs/domains. Wrap them in code spans or Markdown links.
✏️ Example fix
-- OpenO EMR running at http://localhost:8080/oscar
+- OpenO EMR running at `http://localhost:8080/oscar`Also applies to: 98-98, 108-108
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
28-28: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In `@docs/ui-tests/test-2/test-2-README.md` at line 28, Replace the bare URL
"http://localhost:8080/oscar" in the sentence starting with "OpenO EMR running
at" (and the other occurrences at the same lines referenced) with either a code
span or a proper Markdown link; e.g., change the inline bare domain to
`http://localhost:8080/oscar` or
`[http://localhost:8080/oscar](http://localhost:8080/oscar)` so the markdown
linter no longer flags the bare URL.
Add Playwright Chromium path verification tooling
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
This was referenced Jan 18, 2026
This was referenced Jan 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…l demo UI test docs
Summary by cubic
Stabilizes and speeds up CI by splitting modern and legacy test runs and aligning the devcontainer with Playwright 1.56 on Ubuntu 24.04. Adds a full “Test 2” UI test guide and screenshot structure for demo runs.
Refactors
npx playwright install --with-deps; remove manual apt libs for Ubuntu 24.04 t64.make installfor builds and targeted test commands.@playwright/mcp@latestwith updated Chromium path.New Features
Written for commit 4257d85. Summary will update on new commits.
Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.