Skip to content

docs(testing): align guides with the current Playwright workflow#5941

Open
haoyu-haoyu wants to merge 2 commits into
OHIF:masterfrom
haoyu-haoyu:docs/testing-docs-cleanup
Open

docs(testing): align guides with the current Playwright workflow#5941
haoyu-haoyu wants to merge 2 commits into
OHIF:masterfrom
haoyu-haoyu:docs/testing-docs-cleanup

Conversation

@haoyu-haoyu
Copy link
Copy Markdown

@haoyu-haoyu haoyu-haoyu commented Apr 3, 2026

Context

The testing docs had drifted from the way the repo is actually wired today. The main testing page still pointed contributors toward Cypress, while the repository-level end-to-end workflow now runs through Playwright. The root README also had a couple of small inconsistencies that make first-time navigation harder than it needs to be.

Changes & Results

I updated the main testing guide so it points contributors to the current Playwright workflow, links to the dedicated Playwright page, and frames the older package-local Cypress commands as legacy rather than the default path.

I also refreshed the Playwright guide so the commands, report viewer, screenshot notes, and manual server instructions match the current config in the repo.

Finally, I cleaned up the root README by removing a duplicated newsletter block, fixing the community link, and replacing the outdated repository tree with a current overview of the main packages and test directories.

Testing

I ran git diff --check.

I also tried to run a prettier check on the edited files, but the local npm cache in ~/.npm has a permissions problem on this machine, so that validation did not complete.

Checklist

PR

  • My Pull Request title is descriptive, accurate and follows the semantic-release format and guidelines.

Code

  • My changes are documented where they need to be.

Public Documentation Updates

  • The documentation has been updated to reflect the public-facing changes in this pull request.

Tested Environment

  • OS: macOS 26.4
  • Node version: 22.21.1
  • Browser: Not applicable (documentation-only change)

Greptile Summary

This PR aligns OHIF's testing documentation with the current Playwright-based workflow, refreshes the root README, and bumps the Node base image to 20.19.0-slim across all Dockerfiles. The documentation changes are broadly accurate and well-structured, but one typo was introduced in testing.md that would cause the test-data install command to fail.

Key changes:

  • testing.md: Replaces the Cypress-centric e2e instructions with a concise Playwright quick-start; frames legacy Cypress commands correctly as package-local/legacy.
  • playwright-testing.md: Switches all commands from bun to yarn/npx, narrows the screenshot path description to match the current chromium-only default in playwright.config.ts, and adds an accurate manual server snippet using OHIF_PORT=3335.
  • README.md: Removes a duplicated newsletter block, fixes the bare community.ohif.org href to https://community.ohif.org, and replaces a stale (and partly duplicated) directory tree with a clean overview that matches the actual repo layout.
  • All Dockerfiles: Minor Node image bump from 20.18.1-slim20.19.0-slim.
  • Issue found: testing.md line 82 changed the DICOM conversion tool install command from dicomp10-to-dicomweb (the real npm package) to dicom10-to-dicomweb (which does not exist on the npm registry). This would cause the command to fail for any contributor trying to add new test data.

Confidence Score: 4/5

Safe to merge after fixing the dicom10-to-dicomwebdicomp10-to-dicomweb typo in testing.md.

All changes are documentation and Docker image bump — no runtime code is touched. The only issue is a single incorrect npm package name in the test-data section of testing.md that would break the npm install command for contributors adding new test data. Everything else cross-checks cleanly against playwright.config.ts and package.json.

platform/docs/docs/development/testing.md — contains the incorrect dicom10-to-dicomweb package name at line 82.

Important Files Changed

Filename Overview
platform/docs/docs/development/testing.md Redirects e2e section to Playwright workflow correctly, but introduces a typo — dicom10-to-dicomweb should be dicomp10-to-dicomweb (the actual npm package).
platform/docs/docs/development/playwright-testing.md Updated commands from bun to yarn/npx, aligned screenshot path with actual playwright.config.ts (chromium-only), and refreshed manual server instructions — all accurate against the config.
README.md Removes duplicate newsletter block, fixes bare community.ohif.org URL to include https://, and replaces stale/duplicate directory tree with a clean, accurate overview matching the current repo layout.
Dockerfile Bumps Node base image from 20.18.1-slim to 20.19.0-slim; no logic change.
platform/app/.recipes/Nginx-Dcm4chee-Keycloak/dockerfile Bumps Node base image from 20.18.1-slim to 20.19.0-slim to match root Dockerfile.
platform/app/.recipes/Nginx-Dcm4chee/dockerfile Bumps Node base image from 20.18.1-slim to 20.19.0-slim.
platform/app/.recipes/Nginx-Orthanc-Keycloak/dockerfile Bumps Node base image from 20.18.1-slim to 20.19.0-slim.
platform/app/.recipes/Nginx-Orthanc/dockerfile Bumps Node base image from 20.18.1-slim to 20.19.0-slim.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([Contributor]) --> B[yarn test:data\ngit submodule update]
    B --> C{Run mode?}
    C -->|CI pipeline| D[yarn test:e2e:ci\nnpx playwright test]
    C -->|Interactive / UI| E[yarn test:e2e:ui\nnpx playwright test --ui]
    C -->|Headed| F[yarn test:e2e:headed]
    C -->|Debug| G[yarn test:e2e:debug]
    C -->|Manual server| H["npx cross-env APP_CONFIG=config/e2e.js\nOHIF_PORT=3335 yarn start\n→ localhost:3335"]
    H --> I[Playwright reuses\nexisting server]
    D & E & F & G & I --> J{Tests pass?}
    J -->|No - screenshots missing| K[Playwright auto-generates\ntests/screenshots/chromium/…]
    K --> L[Verify ground-truth\nthen re-run]
    L --> J
    J -->|Yes| M[npx playwright show-report\ntests/playwright-report]
Loading

Reviews (1): Last reviewed commit: "Clarify the test docs and refresh the re..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Update base image from node:20.18.1-slim to node:20.19.0-slim across
all Dockerfiles. lerna@9.0.4 requires Node ^20.19.0, so the previous
version caused yarn install to fail during Docker builds.

Fixes OHIF#5860

Signed-off-by: haoyu-haoyu <haoyu-haoyu@users.noreply.github.com>
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 3, 2026

Deploy Preview for ohif-dev ready!

Name Link
🔨 Latest commit 85e8970
🔍 Latest deploy log https://app.netlify.com/projects/ohif-dev/deploys/69d038c305e56c00083995a7
😎 Deploy Preview https://deploy-preview-5941--ohif-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.


![e2e-cypress-final](../assets/img/e2e-cypress-final.png)
```bash
npm install -g dicom10-to-dicomweb
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Incorrect npm package name

The PR changed dicomp10-to-dicomweb (the real package on npm/GitHub at chafey/dicomp10-to-dicomweb-js) to dicom10-to-dicomweb, which does not exist on the npm registry. Running this command will fail with a "package not found" error. The original spelling in the file being replaced was correct.

Suggested change
npm install -g dicom10-to-dicomweb
npm install -g dicomp10-to-dicomweb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant