Playwright Test - Login with HA enabled #1228
Conversation
Signed-off-by: Triona Doyle <tekton@example.com>
Signed-off-by: Triona Doyle <tekton@example.com>
Signed-off-by: Triona Doyle <tekton@example.com>
Signed-off-by: Triona Doyle <tekton@example.com>
openshift-ci
Bot
added
kind/enhancement
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds Playwright coverage for local and OpenShift SSO login during HA deployments, improves HA setup and teardown handling, hardens the UI test runner against stalled or failed setup commands, and adjusts login and locator timing. ChangesUI end-to-end validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Playwright
participant OpenShiftCluster
participant ArgoCDUI
Playwright->>OpenShiftCluster: Enable HA and wait for rollouts
Playwright->>OpenShiftCluster: Extract admin password
Playwright->>ArgoCDUI: Submit local admin login
Playwright->>ArgoCDUI: Perform OpenShift SSO login
ArgoCDUI-->>Playwright: Navigate to applications
Playwright-->>Playwright: Assert Applications is visible
Playwright->>OpenShiftCluster: Disable HA and wait for availability
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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: 2
🤖 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 `@test/ui-e2e/tests/ha-login.spec.ts`:
- Around line 11-56: Extend the timeout for the HA setup hook in test.beforeAll
so its intentional polling and rollout waits can complete within the possible
multi-minute duration. Use the hook’s testInfo parameter and call
testInfo.setTimeout with a budget covering the retry loop and rollout commands,
while leaving the existing setup behavior unchanged.
- Around line 10-11: Override the inherited authentication state for the
ha-login spec by adding test.use with empty cookies and origins before the
tests, so page.goto('/login?dex=none') always reaches the login form. Keep the
existing login flow and beforeAll setup unchanged.
🪄 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 YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: b7eaf2bf-5a5b-4d57-929a-da1ef7ff0e1b
📒 Files selected for processing (6)
test/ui-e2e/.auth/setup.tstest/ui-e2e/global.setup.tstest/ui-e2e/run-ui-tests.shtest/ui-e2e/src/pages/LoginPage.tstest/ui-e2e/tests/ha-login.spec.tstest/ui-e2e/tests/resource-tree.spec.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
argoproj-labs/argocd-operator(manual)
f9ec954 to
1cd2f7e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/ui-e2e/tests/ha-login.spec.ts`:
- Around line 58-72: Update the test.afterAll cleanup hook to wait for every
HA-related resource to finish rolling back, including the redis-ha statefulset,
redis-ha-haproxy, openshift-gitops-server, and dex-server, mirroring the
beforeAll checks. Add an explicit hook timeout covering the combined patch and
wait operations, while preserving the existing cleanup error handling and
success message.
🪄 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 YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: b7ca0596-0444-4584-94c8-372c23e57c79
📒 Files selected for processing (5)
test/ui-e2e/.auth/setup.tstest/ui-e2e/run-ui-tests.shtest/ui-e2e/src/pages/LoginPage.tstest/ui-e2e/tests/ha-login.spec.tstest/ui-e2e/tests/resource-tree.spec.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
argoproj-labs/argocd-operator(manual)
🚧 Files skipped from review as they are similar to previous changes (4)
- test/ui-e2e/tests/resource-tree.spec.ts
- test/ui-e2e/src/pages/LoginPage.ts
- test/ui-e2e/.auth/setup.ts
- test/ui-e2e/run-ui-tests.sh
Signed-off-by: Triona Doyle <tekton@example.com>
1cd2f7e to
2c94e1f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test/ui-e2e/tests/ha-login.spec.ts (1)
70-75: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not treat deletion timeouts as successful cleanup.
A stuck StatefulSet deletion throws after 300s, is swallowed here, skips the HAProxy wait, and still logs a restored cluster. Wait for both resources independently; ignore only confirmed
NotFoundresponses and rethrow timeouts/API failures.🤖 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 `@test/ui-e2e/tests/ha-login.spec.ts` around lines 70 - 75, Update the cleanup try/catch block to wait for the StatefulSet and HAProxy deployment independently, so failure of the first wait does not skip the second. In the cleanup logic, ignore only confirmed NotFound deletion responses; rethrow timeout and other API errors, and only log the cluster as restored after both resources have been handled successfully.Source: Path instructions
🤖 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 `@test/ui-e2e/tests/ha-login.spec.ts`:
- Line 19: Add a finite timeout to both execSync oc patch calls in
test/ui-e2e/tests/ha-login.spec.ts at lines 19 and 63, using the execSync
options so unavailable API servers cannot block the Playwright worker
indefinitely; preserve the existing patch commands and inherited stdio behavior.
---
Duplicate comments:
In `@test/ui-e2e/tests/ha-login.spec.ts`:
- Around line 70-75: Update the cleanup try/catch block to wait for the
StatefulSet and HAProxy deployment independently, so failure of the first wait
does not skip the second. In the cleanup logic, ignore only confirmed NotFound
deletion responses; rethrow timeout and other API errors, and only log the
cluster as restored after both resources have been handled successfully.
🪄 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 YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 9c2bcc97-539e-4bbb-8b1f-b477fe317e6e
📒 Files selected for processing (5)
test/ui-e2e/.auth/setup.tstest/ui-e2e/run-ui-tests.shtest/ui-e2e/src/pages/LoginPage.tstest/ui-e2e/tests/ha-login.spec.tstest/ui-e2e/tests/resource-tree.spec.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
argoproj-labs/argocd-operator(manual)
🚧 Files skipped from review as they are similar to previous changes (4)
- test/ui-e2e/tests/resource-tree.spec.ts
- test/ui-e2e/src/pages/LoginPage.ts
- test/ui-e2e/run-ui-tests.sh
- test/ui-e2e/.auth/setup.ts
Signed-off-by: Triona Doyle <tekton@example.com>
Signed-off-by: Triona Doyle <tekton@example.com>
1 participant
What type of PR is this?
/kind enhancement
/kind cleanup
What does this PR do / why we need it:
This PR introduces a new E2E test suite to verify UI login functionality when Argo CD is configured with High Availability (HA) enabled.
Alongside the new test, it hardens the existing Playwright E2E framework
Improved error handling, timeout tuning, and UI locators to reduce test flakiness.
Cleaned up the
run-ui-tests.shexecution script and added strict fail checks.Have you updated the necessary documentation?
Which issue(s) this PR fixes:
Fixes GITOPS-102920
Test acceptance criteria:
How to test changes / Special notes to the reviewer:
Verified on
Run the new test locally against any provisioned cluster:
./test/ui-e2e/run-ui-tests.sh tests/ha-login.spec.ts --headed --reporter=list --trace on --project=chromium