CONSOLE-5416: Migrate dev-console Cypress tests to Playwright (batch 3b)#16774
CONSOLE-5416: Migrate dev-console Cypress tests to Playwright (batch 3b)#16774shahsahil264 wants to merge 3 commits into
Conversation
Migrates 4 dev-console Cypress feature files to Playwright: add-page, import-from-git, import-from-devfile, quick-search Includes shared page objects and infrastructure changes (add-page.ts, base-page.ts, kubernetes-client.ts) identical to batch 3a for independent CI testing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
@shahsahil264: This pull request references CONSOLE-5416 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 sub-task 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. |
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: shahsahil264 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 |
WalkthroughThis change migrates Developer Console Add-page, Git/devfile import, topology, and quick-search coverage to Playwright page objects and E2E specs. It also updates shared perspective selectors and ConfigMap patching used by the tests. ChangesDeveloper Console E2E migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Test as Playwright test
participant Add as AddPage
participant Git as ImportFromGitPage
participant Topology as TopologyPage
Test->>Add: Open Import from Git
Add->>Git: Navigate to import form
Test->>Git: Enter repository and workload name
Git->>Git: Validate and create workload
Test->>Topology: Find created workload
Topology-->>Test: Return workload and sidebar title
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ 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.
Actionable comments posted: 4
🤖 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 `@frontend/e2e/clients/kubernetes-client.ts`:
- Around line 380-381: Update the patch request around the ConfigMap data
mutation to use the JSON Patch operation “add” for “/data” instead of “replace”,
and set the patch options’ Content-Type to
k8s.PatchUtils.PATCH_FORMAT_JSON_PATCH.
In `@frontend/e2e/tests/dev-console/import-from-devfile.spec.ts`:
- Around line 43-50: In the “Fill out the devfile import form” step, verify that
the Devfile import strategy is selected after git validation and before calling
gitPage.clickCreate(). Use the existing strategy-selection assertion or selector
exposed by gitPage, ensuring the test fails if another import strategy is
active.
In `@frontend/e2e/tests/dev-console/import-from-git.spec.ts`:
- Around line 72-76: Extend the “Verify workload in topology” test step to query
the workload’s namespace and assert that no Route exists for name-no-route. Keep
the existing workload and sidebar checks, and use the test’s established
namespace/resource-query utilities.
In `@frontend/e2e/tests/dev-console/quick-search.spec.ts`:
- Around line 44-50: Replace the generic heading assertion after clicking
viewAllLink in the quick-search test with a catalog-specific URL or heading
assertion. In frontend/e2e/tests/dev-console/import-from-git.spec.ts lines
51-53, replace the generic post-navigation heading check with an assertion for
/add/ns/${ns} or the Add-page heading, ensuring both tests verify the intended
destination.
🪄 Autofix (Beta)
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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d6a6282c-2a15-4275-806b-d5c43e305eaf
📒 Files selected for processing (11)
frontend/e2e/clients/kubernetes-client.tsfrontend/e2e/pages/base-page.tsfrontend/e2e/pages/dev-console/add-page.tsfrontend/e2e/tests/dev-console/add-page.spec.tsfrontend/e2e/tests/dev-console/import-from-devfile.spec.tsfrontend/e2e/tests/dev-console/import-from-git.spec.tsfrontend/e2e/tests/dev-console/quick-search.spec.tsfrontend/packages/dev-console/integration-tests/features/addFlow/add-page.featurefrontend/packages/dev-console/integration-tests/features/addFlow/create-from-devfile.featurefrontend/packages/dev-console/integration-tests/features/addFlow/create-from-git.featurefrontend/packages/dev-console/integration-tests/features/addFlow/quick-search-add.feature
💤 Files with no reviewable changes (4)
- frontend/packages/dev-console/integration-tests/features/addFlow/quick-search-add.feature
- frontend/packages/dev-console/integration-tests/features/addFlow/create-from-git.feature
- frontend/packages/dev-console/integration-tests/features/addFlow/create-from-devfile.feature
- frontend/packages/dev-console/integration-tests/features/addFlow/add-page.feature
| body: [{ op: 'replace', path: '/data', value: mergedData }], | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C4 \
'patchNamespacedConfigMap|PatchStrategy\.(JsonPatch|MergePatch)|setHeaderOptions' \
frontendRepository: openshift/console
Length of output: 942
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- package references ---\n'
rg -n '"`@kubernetes/client-node`"|client-node' package.json frontend package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null || true
printf '\n--- nearby kubernetes-client code ---\n'
sed -n '360,390p' frontend/e2e/clients/kubernetes-client.ts
printf '\n--- patch method usages in repo ---\n'
rg -n 'patchNamespacedConfigMap\(|contentType:\s*k8s\.PatchStrategy|PatchStrategy\.' frontend -g '*.ts' -g '*.js' || trueRepository: openshift/console
Length of output: 246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- package references ---'
rg -n '"`@kubernetes/client-node`"|client-node' package.json frontend package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null || true
echo
echo '--- nearby kubernetes-client code ---'
sed -n '360,390p' frontend/e2e/clients/kubernetes-client.ts
echo
echo '--- patch method usages in repo ---'
rg -n 'patchNamespacedConfigMap\(|contentType:\s*k8s\.PatchStrategy|PatchStrategy\.' frontend -g '*.ts' -g '*.js' || trueRepository: openshift/console
Length of output: 1650
🌐 Web query:
@kubernetes/client-node patchNamespacedConfigMap body array contentType JsonPatch 1.4.0
💡 Result:
When using @kubernetes/client-node to perform a patch operation on a ConfigMap (or other resources) using the JsonPatch format (JSON Patch, RFC 6902), the generated client often does not automatically set the required Content-Type header [1]. This frequently results in a 415 Unsupported Media Type error from the Kubernetes API server [2][1]. To resolve this, you must explicitly pass the Content-Type header via the options parameter in your patchNamespacedConfigMap call [3][2]. The correct Content-Type for JSON Patch is application/json-patch+json [3]. You can access this constant directly from the library's PatchUtils [3]. Example Usage: const k8s = require('@kubernetes/client-node'); const coreApi = k8s.ConfigFactory.defaultConfig.makeApiClient(k8s.CoreV1Api); // Define your JSON patch operations as an array const patch = [ { op: 'replace', path: '/data/key1', value: 'newValue' } ]; // Set the correct content-type header const options = { headers: { 'Content-Type': k8s.PatchUtils.PATCH_FORMAT_JSON_PATCH } }; // Perform the patch await coreApi.patchNamespacedConfigMap( 'configmap-name', 'namespace', patch, undefined, undefined, undefined, undefined, options); Key Implementation Details: 1. Header Requirement: The Kubernetes API server requires the specific Content-Type corresponding to the patch strategy [3][1]. 2. Patch Format: Use k8s.PatchUtils.PATCH_FORMAT_JSON_PATCH (which maps to application/json-patch+json) when providing the body as an array of operations [3]. 3. Method Signature: The options object is typically the final argument in the patchNamespacedConfigMap method [4][2]. Ensure all preceding optional arguments (such as pretty, dryRun, fieldManager, and force) are explicitly set to undefined if they are not being used [2][1].
Citations:
- 1: Add an example of how to use patch. kubernetes-client/javascript#450
- 2: 415 Unsupported Media Type Error when Patching Node kubernetes-client/javascript#995
- 3: https://stackoverflow.com/questions/67520468/patch-k8s-custom-resource-with-kubernetes-client-node
- 4: Expose TCP connect() and recv() timeouts for HTTP client kubernetes-client/javascript#544
🌐 Web query:
@kubernetes/client-node PatchUtils PATCH_FORMAT_JSON_PATCH patchNamespacedConfigMap options headers Content-Type
💡 Result:
In the @kubernetes/client-node library, the patchNamespacedConfigMap function (and other similar patch operations) often requires explicitly setting the Content-Type header to match the specific patch format being used, as the default behavior may not automatically select the correct type or might lead to a 415 Unsupported Media Type error [1][2]. You can manage these headers by passing an options object as the final argument to the function [1][2]. This options object should contain a headers property with the desired Content-Type [3][4]. The library provides a PatchUtils class that defines the appropriate strings for the various patch formats supported by the Kubernetes API [5][6]: - k8s.PatchUtils.PATCH_FORMAT_JSON_PATCH (application/json-patch+json) [3][5] - k8s.PatchUtils.PATCH_FORMAT_JSON_MERGE_PATCH (application/merge-patch+json) [3][5] - k8s.PatchUtils.PATCH_FORMAT_STRATEGIC_MERGE_PATCH (application/strategic-merge-patch+json) [3][5] - k8s.PatchUtils.PATCH_FORMAT_APPLY_YAML (application/apply-patch+yaml) [5][6] To perform a patch, construct your options object as follows: const options = { headers: { 'Content-Type': k8s.PatchUtils.PATCH_FORMAT_JSON_PATCH } }; Then, pass this object as the last argument in your API call. Note that depending on the version of the @kubernetes/client-node library you are using, the number of positional arguments (such as undefined for unused parameters like dryRun or fieldManager) may vary [1][2]. Example usage: await k8sApi.patchNamespacedConfigMap( name, namespace, body, undefined, // dryRun undefined, // fieldManager undefined, // fieldValidation undefined, // force options // options with headers); Always ensure your patch body is formatted correctly to correspond with the Content-Type you have selected [3].
Citations:
- 1: Missing Content-Type header on patch operation kubernetes-client/javascript#19
- 2: 415 Unsupported Media Type Error when Patching Node kubernetes-client/javascript#995
- 3: https://stackoverflow.com/questions/67520468/patch-k8s-custom-resource-with-kubernetes-client-node
- 4: How to update configMap with appended data? kubernetes-client/javascript#489
- 5: https://app.unpkg.com/@kubernetes/client-node@0.20.0/files/dist/patch.d.ts
- 6: https://app.unpkg.com/@kubernetes/client-node@0.19.0/files/dist/patch.d.ts
🌐 Web query:
@kubernetes/client-node 1.4.0 patchNamespacedConfigMap signature options headers contentType PatchUtils
💡 Result:
In @kubernetes/client-node version 1.4.0, the patchNamespacedConfigMap method is part of the CoreV1Api generated client. When using this method to perform a PATCH operation, you must specify the correct Content-Type header to match your patch format [1]. The client does not automatically set the Content-Type header for non-standard patch operations [2][3][4]. You should pass an options object as the final argument to the function containing your headers [2][1][4]. You can use the PatchUtils utility class provided by the library to set the appropriate Content-Type [1][5]: import { PatchUtils } from '@kubernetes/client-node'; // Example: Using JSON Patch const options = { headers: { 'Content-Type': PatchUtils.PATCH_FORMAT_JSON_PATCH, }, }; // Method signature: // await coreV1Api.patchNamespacedConfigMap( // name, // namespace, // body, // pretty, // dryRun, // fieldManager, // fieldValidation, // force, // options // Pass the options object here //); Available PatchUtils formats include [1][5]: - PatchUtils.PATCH_FORMAT_JSON_PATCH ('application/json-patch+json') - PatchUtils.PATCH_FORMAT_JSON_MERGE_PATCH ('application/merge-patch+json') - PatchUtils.PATCH_FORMAT_STRATEGIC_MERGE_PATCH ('application/strategic-merge-patch+json') - PatchUtils.PATCH_FORMAT_APPLY_YAML ('application/apply-patch+yaml') Note: Because generated API methods in this library frequently receive new parameters (such as fieldValidation), the exact position of the options object in the argument list can shift between versions [2][3]. Always verify the current argument signature for your specific library version in your IDE's type definitions or the generated code in dist/gen/api/coreV1Api.ts [6][4]. In version 1.4.0, ensure your options object is passed as the final argument [2].
Citations:
- 1: https://stackoverflow.com/questions/67520468/patch-k8s-custom-resource-with-kubernetes-client-node
- 2: patchNode and patchNamespacedService 415 UnsupportedMediaType Errors kubernetes-client/javascript#862
- 3: 415 Unsupported Media Type Error when Patching Node kubernetes-client/javascript#995
- 4: Test and Replace patch fails but works with kubectl kubernetes-client/javascript#2095
- 5: https://app.unpkg.com/@kubernetes/client-node@0.20.0/files/dist/patch.d.ts
- 6: https://app.unpkg.com/@kubernetes/client-node@1.4.0/files/dist/gen/index.d.ts
Use add /data and set the JSON Patch content type here. replace fails when a ConfigMap has no data field, and this request needs Content-Type: k8s.PatchUtils.PATCH_FORMAT_JSON_PATCH in the patch options.
🤖 Prompt for 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.
In `@frontend/e2e/clients/kubernetes-client.ts` around lines 380 - 381, Update the
patch request around the ConfigMap data mutation to use the JSON Patch operation
“add” for “/data” instead of “replace”, and set the patch options’ Content-Type
to k8s.PatchUtils.PATCH_FORMAT_JSON_PATCH.
| await test.step('Click view all and verify catalog', async () => { | ||
| const viewAllLink = addPage.getViewAllLink(); | ||
| await expect(viewAllLink).toBeVisible({ timeout: 10_000 }); | ||
| await viewAllLink.click(); | ||
| const catalogPage = new SoftwareCatalogPage(page); | ||
| await expect(catalogPage.getPageHeading()).toBeVisible({ timeout: 15_000 }); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use destination-specific assertions after navigation.
Both tests assert a generic heading already present on the source page, allowing false positives when navigation fails.
frontend/e2e/tests/dev-console/quick-search.spec.ts#L44-L50: assert the catalog URL or catalog-specific heading.frontend/e2e/tests/dev-console/import-from-git.spec.ts#L51-L53: assert/add/ns/${ns}or the Add-page heading text.
📍 Affects 2 files
frontend/e2e/tests/dev-console/quick-search.spec.ts#L44-L50(this comment)frontend/e2e/tests/dev-console/import-from-git.spec.ts#L51-L53
🤖 Prompt for 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.
In `@frontend/e2e/tests/dev-console/quick-search.spec.ts` around lines 44 - 50,
Replace the generic heading assertion after clicking viewAllLink in the
quick-search test with a catalog-specific URL or heading assertion. In
frontend/e2e/tests/dev-console/import-from-git.spec.ts lines 51-53, replace the
generic post-navigation heading check with an assertion for /add/ns/${ns} or the
Add-page heading, ensuring both tests verify the intended destination.
- patchConfigMap: use JSON Patch op "add" instead of "replace" so it works when /data key doesn't exist yet - import-from-devfile: assert Devfile strategy is selected before Create - quick-search: verify catalog URL after "View all" click - Remove unused Page type import from add-page.ts Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/test e2e-gcp-console |
TC03: verify cancel redirects to /add/ns/ URL, not just a generic heading. TC04: verify no Route resource exists after creating workload without route. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/test e2e-gcp-console |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
@shahsahil264: 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. |
2 participants
Analysis / Root cause:
Part of the Cypress → Playwright migration epic (CONSOLE-5196). This is the second half of batch 3, split from PR #16751 to keep CI test counts under the auth session timeout.
Solution description:
Migrates 4 dev-console Cypress feature files to idiomatic Playwright:
add-page.feature→add-page.spec.ts(7 tests)create-from-git.feature→import-from-git.spec.ts(4 tests)create-from-devfile.feature→import-from-devfile.spec.ts(1 test)quick-search-add.feature→quick-search.spec.ts(4 tests)Includes shared page objects and infrastructure changes (add-page.ts, base-page.ts, kubernetes-client.ts) identical to batch 3a (#16751) for independent CI testing. When #16751 merges first, conflicts on shared files resolve by accepting main.
Screenshots / screen recording:
N/A — E2E test migration, no UI changes.
Test setup:
Requires an OpenShift cluster with Developer perspective enabled.
Test cases:
Browser conformance:
Merge criteria:
Summary by CodeRabbit