CONSOLE-5211: Add Cypress-to-Playwright migration skills and context#16315
CONSOLE-5211: Add Cypress-to-Playwright migration skills and context#16315stefanonardo wants to merge 1 commit intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@stefanonardo: This pull request references CONSOLE-5211 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
stefanonardo
force-pushed
the
CONSOLE-5211
branch
from
db8bb42 to
582ce1e
Compare
📝 WalkthroughWalkthroughThis pull request adds documentation and Claude skill definitions to support migration of Console's e2e test suite from Cypress to Playwright. New files include a migration-context document that defines guiding principles, selector translations, and structural transformation rules; two skill definitions ( 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.claude/skills/debug-test/SKILL.md (2)
25-25: Optional: Add language specifiers to fenced code blocks.Static analysis flagged that the code blocks at lines 25 and 74 are missing language specifiers, which can affect syntax highlighting and rendering.
🔧 Add language hints for better rendering
-``` +```text /debug-test e2e/tests/cluster-settings/upstream-modal.spec.ts /debug-test e2e/tests/cluster-settings/ --workers=2 /debug-test `@admin`And for line 74: ```diff -``` +```text Debug Summary: <target> Total tests: NThis improves markdown rendering but doesn't affect functionality.
Also applies to: 74-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/debug-test/SKILL.md at line 25, Add language specifiers to the two fenced code blocks in SKILL.md (the block showing the debug-test command lines and the block showing the "Debug Summary" snippet) so they render correctly; update the opening ``` fences to include an appropriate language hint like "text" (e.g., change ``` to ```text) for the code block containing "/debug-test e2e/tests/..." and the block containing "Debug Summary: <target> Total tests: N" (look for the fenced code blocks near the examples referenced in the review).
88-101: Practical troubleshooting for realistic failure modes.The troubleshooting section correctly distinguishes between test bugs, infrastructure issues, and flakiness. The guidance on flaky tests — use MCP to identify the timing-sensitive interaction and fix with proper waits/clicks rather than masking with retries — is essential.
✍️ Optional: Address static analysis adverb placement suggestion
The static analysis tool suggests moving the adverbs before the verbs for better style:
-### Flaky test (passes sometimes, fails sometimes) +### Flaky test (sometimes passes, sometimes fails)This is a minor style improvement and entirely optional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/debug-test/SKILL.md around lines 88 - 101, Move optional adverbs so they appear before the verbs to satisfy the static analysis style suggestion in the Troubleshooting section of SKILL.md; e.g., change phrasing like "diagnose from error messages alone" to "diagnose alone from error messages" or "focus on fixes that don't require live inspection" to "focus on fixes that don't require live inspection" as appropriate—apply this consistently to sentences in the "Playwright MCP not connected" and "Flaky test (passes sometimes, fails sometimes)" paragraphs, preserving meaning and approved content.AGENTS.md (1)
125-128: Clear integration point for Playwright migration workflow.The new section effectively directs users to the right skills and shared context. The placement and cross-references are correct.
📝 Optional: Consider adding brief context about the migration scope
If you'd like to provide additional context for future contributors, you could mention the scope or timeline of the Cypress→Playwright migration:
## Playwright migration -We are migrating Cypress e2e tests to Playwright. Use `/migrate-cypress` to convert test files and `/debug-test` to fix failing tests. Shared migration context (translation tables, structural rules, checklist) is in `.claude/migration-context.md`. +Console is migrating its e2e test suite from Cypress to Playwright. Use `/migrate-cypress` to convert test files and `/debug-test` to fix failing tests. Shared migration context (translation tables, structural rules, checklist) is in `.claude/migration-context.md`.This is purely optional — the current version is perfectly functional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 125 - 128, Add a short sentence under the "## Playwright migration" heading that clarifies the migration scope and expectations (for example, which test suites are in-scope and a rough timeline or milestone), referencing the existing commands /migrate-cypress and /debug-test and the shared context file .claude/migration-context.md so contributors know where to find translation tables and rules; keep the addition one concise line to avoid changing placement or cross-references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.claude/skills/debug-test/SKILL.md:
- Line 25: Add language specifiers to the two fenced code blocks in SKILL.md
(the block showing the debug-test command lines and the block showing the "Debug
Summary" snippet) so they render correctly; update the opening ``` fences to
include an appropriate language hint like "text" (e.g., change ``` to ```text)
for the code block containing "/debug-test e2e/tests/..." and the block
containing "Debug Summary: <target> Total tests: N" (look for the fenced code
blocks near the examples referenced in the review).
- Around line 88-101: Move optional adverbs so they appear before the verbs to
satisfy the static analysis style suggestion in the Troubleshooting section of
SKILL.md; e.g., change phrasing like "diagnose from error messages alone" to
"diagnose alone from error messages" or "focus on fixes that don't require live
inspection" to "focus on fixes that don't require live inspection" as
appropriate—apply this consistently to sentences in the "Playwright MCP not
connected" and "Flaky test (passes sometimes, fails sometimes)" paragraphs,
preserving meaning and approved content.
In `@AGENTS.md`:
- Around line 125-128: Add a short sentence under the "## Playwright migration"
heading that clarifies the migration scope and expectations (for example, which
test suites are in-scope and a rough timeline or milestone), referencing the
existing commands /migrate-cypress and /debug-test and the shared context file
.claude/migration-context.md so contributors know where to find translation
tables and rules; keep the addition one concise line to avoid changing placement
or cross-references.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e1425cb7-534e-4d4e-b410-deba32856448
📒 Files selected for processing (4)
.claude/migration-context.md.claude/skills/debug-test/SKILL.md.claude/skills/migrate-cypress/SKILL.mdAGENTS.md
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-04-22T12:54:27.313Z
Learning: Run `yarn i18n` after adding translatable strings and commit updated i18n keys alongside code changes that affect i18n
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-04-22T12:54:27.313Z
Learning: Backend dependency updates should be in separate commits from core logic changes to isolate vendor folder changes
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-04-22T12:54:27.313Z
Learning: Bug fix commits should be prefixed with bug number or Jira key (e.g., OCPBUGS-1234: Fix ...)
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-04-22T12:54:27.313Z
Learning: Commit subject lines should answer 'what changed' and body should answer 'why'
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-04-22T12:54:27.313Z
Learning: Feature branch names should follow the format CONSOLE-#### (Jira story number)
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-04-22T12:54:27.313Z
Learning: Bug fix branch names should follow the format OCPBUGS-#### (Jira bug number)
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-04-22T12:54:27.313Z
Learning: Before making changes to the dynamic plugin SDK, ensure changes do not impact the public API by consulting the plugin-api-review skill and checking for breaking changes
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-04-22T12:54:27.313Z
Learning: Always consult TESTING.md before writing or modifying tests to ensure compliance with Jest, Cypress, and Go testing patterns and best practices
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-04-22T12:54:27.313Z
Learning: Always consult STYLEGUIDE.md when writing new code or reviewing style questions for TypeScript, Go, and SCSS conventions
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-04-22T12:54:27.313Z
Learning: Always consult INTERNATIONALIZATION.md when adding or modifying user-facing strings for i18n patterns and useTranslation usage
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-04-22T12:54:27.313Z
Learning: Maintain backward compatibility in the dynamic plugin SDK as it is a public API consumed by external plugins
Learnt from: CR
Repo: openshift/console
Timestamp: 2026-04-22T12:54:27.313Z
Learning: Provide comprehensive documentation for all public APIs in the dynamic plugin SDK
🪛 LanguageTool
.claude/skills/debug-test/SKILL.md
[style] ~99-~99: The adverb ‘sometimes’ is usually put before the verb ‘passes’.
Context: ...structure setup first. ### Flaky test (passes sometimes, fails sometimes) If a test passes on r...
(ADVERB_WORD_ORDER)
[style] ~99-~99: The adverb ‘sometimes’ is usually put before the verb ‘fails’.
Context: ...rst. ### Flaky test (passes sometimes, fails sometimes) If a test passes on re-run without any...
(ADVERB_WORD_ORDER)
🪛 markdownlint-cli2 (0.22.0)
.claude/skills/debug-test/SKILL.md
[warning] 25-25: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 74-74: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (13)
AGENTS.md (1)
24-40: Nice cleanup — formatting now matches repo conventions.The addition of the blank line, trailing comma in the JSON example, and consistent use of double quotes in the TypeScript snippet align well with the repository's style guidelines.
.claude/skills/migrate-cypress/SKILL.md (3)
32-54: Well-structured phased approach with pragmatic MCP fallback.The analysis phase correctly emphasizes understanding intent before translating code, and the MCP-based selector discovery with graceful degradation when unavailable is a solid design choice.
56-95: Implementation and validation phases follow best practices.The page object example demonstrates the correct pattern extending
BasePage, and the validation workflow appropriately includes type checking and linting before test execution. The migration summary format provides clear accountability.
96-124: Solid translation principles and practical troubleshooting guidance.The emphasis on understanding intent over transliteration, eliminating fixed waits, and using framework components first are all critical for producing maintainable Playwright tests. The troubleshooting section addresses realistic failure modes.
.claude/migration-context.md (6)
1-29: Strong foundational principles and accurate selector mappings.The five principles correctly prioritize understanding intent, test isolation, idiomatic API usage, framework reuse, and live verification. The selector command table comprehensively covers Console's custom commands with accurate Playwright equivalents.
30-119: Comprehensive and accurate API translation reference.The translation tables correctly map Cypress patterns to idiomatic Playwright equivalents. Particularly notable are the strong guidance against
waitForTimeout(), the emphasis onrobustClick()for PatternFly interactions, and the replacement of shell commands withKubernetesClientAPI calls.
120-210: Excellent structural transformation guidance with clear examples.Rules 1-4 effectively demonstrate the paradigm shift from Cypress's shared state model to Playwright's isolated tests. The code examples correctly use
test.step,cleanupfixtures, page object methods, and condition-based waits.
212-318: Complete structural rules and well-reasoned isolation strategies.Rules 5-8 correctly address shell command replacement, scoping patterns, conditional logic, and retry configuration. The three isolation strategies provide clear decision criteria: prefer self-contained (A), use shared setup (B) only for expensive read-only resources, and leverage API creation (C) for UI-focused tests.
319-394: Clear page object pattern and comprehensive file mapping.The page object example correctly demonstrates
BasePageextension, locator definition patterns, and the documented locator priority. The Gherkin collapse guidance correctly maps the 4-file indirection to Playwright's more direct structure, and the file mapping table provides clear migration paths.
395-421: Essential checklist and critical "never do" constraints.The migration checklist covers the complete workflow systematically. The "never do" list correctly identifies anti-patterns that would produce fragile or non-idiomatic tests — particularly the requirement to import from
e2e/fixturesrather than@playwright/test(ensures custom fixtures are available) and the prohibition onwaitForTimeout()(ensures reliability)..claude/skills/debug-test/SKILL.md (3)
32-53: Methodical diagnosis workflow with appropriate stability verification.The run phase correctly uses
--retries=0to expose flakiness, and the stability check for single files (3 consecutive runs) is a pragmatic safeguard. The MCP-based diagnosis systematically gathers viewport, accessibility tree, interaction, and error data before classifying failures.
55-87: Sensible fix prioritization and thorough validation.The priority ordering correctly addresses cascading failures first (imports/types), then uses MCP data for stale selectors, followed by timing and missing awaits. The requirement to verify each fix individually before proceeding prevents accumulating multiple changes that obscure root causes.
102-108: Clear rules for the debug workflow.The rules correctly enforce MCP-first diagnosis, architectural boundaries, strict retry configuration during debugging, and linting after fixes.
stefanonardo
force-pushed
the
CONSOLE-5211
branch
from
582ce1e to
719e154
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: stefanonardo 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 |
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
stefanonardo
force-pushed
the
CONSOLE-5211
branch
from
719e154 to
f0799a5
Compare
|
@stefanonardo: all tests passed! 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. |
3 participants
Summary
/migrate-cypressand/debug-testClaude Code skills for Cypress-to-Playwright test migration.claude/migration-context.mdshared reference (adapted fromcypress-migrator.mdc)AGENTS.mdSummary by CodeRabbit
Documentation
Chores