Skip to content

Commit e8cd40c

Browse files
zdrapelaclaude
andcommitted
refactor: deduplicate fix-e2e skills by removing redundant mapping tables
Skills now reference the e2e-fix-workflow rule for mapping tables (job→branch, job→platform, branch→image repo/tag) instead of duplicating them. This reduces token usage by ~40% while keeping the unique procedural value each skill provides: - parse-ci-failure: structured output template with derivation details, GCS URLs, and flag breakdown table - deploy-rhdh: concrete example command, prominent CLI mode warning - diagnose-and-fix: "Try Healer Agent First" callout, removed duplicated coding conventions (~80 lines) - reproduce-failure: removed duplicated project reference table - submit-and-review: prominent dynamic GITHUB_USER extraction, removed CI check types table Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5d23379 commit e8cd40c

File tree

20 files changed

+544
-924
lines changed

20 files changed

+544
-924
lines changed

.claude/skills/deploy-rhdh/SKILL.md

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,30 +51,30 @@ Vault authentication, cluster service account setup, RHDH deployment, and test e
5151

5252
### CLI Mode (Preferred)
5353

54-
CLI mode requires **all three** flags (`-j`, `-r`, `-t`) to avoid falling into interactive mode:
54+
**CRITICAL**: CLI mode requires **all three** flags (`-j`, `-r`, `-t`). If `-r` is omitted, the script falls into interactive mode and will hang in automated contexts.
5555

5656
```bash
5757
cd e2e-tests
5858
./local-run.sh -j <full-prow-job-name> -r <image-repo> -t <image-tag> [-s]
5959
```
6060

61+
**Example** — deploying for a main branch OCP Helm nightly job (deploy-only):
62+
```bash
63+
cd e2e-tests
64+
./local-run.sh -j periodic-ci-redhat-developer-rhdh-main-e2e-ocp-v4-20-helm-nightly -r rhdh-community/rhdh -t next -s
65+
```
66+
6167
**Parameters:**
6268
- `-j / --job`: The **full Prow CI job name** extracted from the Prow URL. The `openshift-ci-tests.sh` handler uses bash glob patterns (like `*ocp*helm*nightly*`) to match, so the full name works correctly. Example: `periodic-ci-redhat-developer-rhdh-main-e2e-ocp-v4-20-helm-nightly`
6369
- `-r / --repo`: Image repository (**required** for CLI mode — without it the script enters interactive mode)
6470
- `-t / --tag`: Image tag (e.g., `1.9`, `next`)
6571
- `-s / --skip-tests`: Deploy only, skip test execution (useful when you want to run tests manually afterward)
6672

67-
Do NOT use shortened job names like `nightly-ocp-helm` for `-j` — these do not match the glob patterns in `openshift-ci-tests.sh`.
73+
**WARNING**: Do NOT use shortened job names like `nightly-ocp-helm` for `-j` — these do not match the glob patterns in `openshift-ci-tests.sh`.
6874

6975
### Image Selection
7076

71-
Match the image repo and tag to the release branch:
72-
73-
| Release branch | `-r` (image repo) | `-t` (image tag) |
74-
|---------------|-------------------|-------------------|
75-
| `main` | `rhdh-community/rhdh` | `next` |
76-
| `release-1.9` | `rhdh/rhdh-hub-rhel9` | `1.9` |
77-
| `release-1.8` | `rhdh/rhdh-hub-rhel9` | `1.8` |
77+
Refer to the `e2e-fix-workflow` rule for the release branch to image repo/tag mapping table.
7878

7979
### Deploy-Only Mode
8080

.claude/skills/diagnose-and-fix/SKILL.md

Lines changed: 16 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,16 @@ Analyze the root cause of a failing E2E test and implement a fix following RHDH
1212

1313
Use this skill after reproducing a failure (via `reproduce-failure`) when you have confirmed the test fails and need to determine the root cause and implement a fix.
1414

15+
## IMPORTANT: Try the Playwright Healer Agent First
16+
17+
**For locator drift and timing/race condition failures (categories 1 and 2 below), always invoke the Playwright healer agent BEFORE doing manual investigation.** The healer can run the test, inspect the live UI, generate correct locators, and edit the code — often resolving the issue end-to-end without manual intervention.
18+
19+
```
20+
@playwright-test-healer Fix the failing test in playwright/e2e/plugins/<plugin>/<spec-file>.spec.ts
21+
```
22+
23+
Only proceed with manual diagnosis if the healer agent cannot resolve the issue (e.g., the failure involves data dependencies, platform-specific behavior, or deployment configuration problems).
24+
1525
## Failure Pattern Recognition
1626

1727
### 1. Locator Drift
@@ -21,8 +31,8 @@ Use this skill after reproducing a failure (via `reproduce-failure`) when you ha
2131
**Cause**: The UI has changed and selectors no longer match.
2232

2333
**Fix approach**:
24-
- Use the Playwright healer agent (`@playwright-test-healer`) to replay the test, inspect the current UI via page snapshots, and generate updated locators
25-
- Update to semantic selectors (see Coding Conventions below)
34+
- Invoke the Playwright healer agent (`@playwright-test-healer`) — it will replay the test, inspect the current UI via page snapshots, generate updated locators, and edit the code automatically
35+
- If the healer cannot resolve it, manually update to semantic role-based locators (see project rules)
2636
- Verify the updated locator works by re-running the test
2737

2838
### 2. Timing / Race Condition
@@ -32,7 +42,8 @@ Use this skill after reproducing a failure (via `reproduce-failure`) when you ha
3242
**Cause**: Test acts before the UI is ready, or waits are insufficient.
3343

3444
**Fix approach**:
35-
- Replace `page.waitForTimeout()` with proper waits: `expect(locator).toBeVisible()`, `page.waitForLoadState()`
45+
- Invoke the Playwright healer agent first — it can identify timing issues by stepping through the test and observing UI state transitions
46+
- If manual fixes are needed: replace `page.waitForTimeout()` with proper waits: `expect(locator).toBeVisible()`, `page.waitForLoadState()`
3647
- Use `expect().toPass()` with retry intervals for inherently async checks:
3748
```typescript
3849
await expect(async () => {
@@ -123,96 +134,9 @@ Use `@playwright-test-planner` when you need to understand a complex user flow b
123134

124135
Use `@playwright-test-generator` when a test needs major rework and you need to generate new test steps from a plan.
125136

126-
## RHDH Project Coding Conventions
127-
128-
### Locator Best Practices
129-
130-
**Always use semantic (role-based) locators** from `e2e-tests/playwright/support/selectors/semantic-selectors.ts`:
131-
132-
```typescript
133-
// GOOD - semantic selectors
134-
page.getByRole('button', { name: 'Create' })
135-
page.getByRole('heading', { name: 'Catalog' })
136-
page.getByRole('link', { name: 'API' })
137-
page.getByRole('tab', { name: 'Overview' })
138-
page.getByLabel('Search')
139-
page.getByText('No results found')
140-
141-
// BAD - CSS class selectors (deprecated)
142-
page.locator('.MuiButton-root')
143-
page.locator('[data-testid="catalog-page"]')
144-
page.locator('#search-input')
145-
```
146-
147-
Use `SemanticSelectors` helper class methods:
148-
```typescript
149-
import { SemanticSelectors } from '../support/selectors/semantic-selectors';
150-
151-
// Table interactions
152-
SemanticSelectors.getTableRow(page, 'my-entity')
153-
SemanticSelectors.getButton(page, 'Create')
154-
```
155-
156-
### Page Object Model
157-
158-
- Page classes live in `e2e-tests/playwright/support/pages/`
159-
- Selectors live in `e2e-tests/playwright/support/page-objects/`
160-
- Use existing page helpers: `UIhelper`, `Common`, `APIHelper`, `KubeClient`
161-
162-
### Component Annotations
163-
164-
Every spec file must have a component annotation:
165-
166-
```typescript
167-
test.beforeAll(async ({}, testInfo) => {
168-
testInfo.annotations.push({
169-
type: 'component',
170-
description: 'your_component_name',
171-
});
172-
});
173-
```
174-
175-
### Utility Classes
176-
177-
- **`Common`** (`utils/common.ts`): Login flows, `waitForLoad()`, `signOut()`, `setupBrowser()`
178-
- **`UIhelper`** (`utils/ui-helper.ts`): 90+ UI interaction methods (click, verify, navigate)
179-
- **`APIHelper`** (`utils/api-helper.ts`): GitHub API, Backstage catalog API
180-
- **`KubeClient`** (`utils/kube-client.ts`): K8s resource management
181-
- **`RHDHDeployment`** (`utils/authentication-providers/rhdh-deployment.ts`): RHDH deployment lifecycle
182-
183-
### Conditional Test Skipping
184-
185-
```typescript
186-
import { skipIfJobName, skipIfJobType, skipIfIsOpenShift } from '../utils/helper';
187-
import * as constants from '../utils/constants';
188-
189-
// Skip on specific platforms
190-
skipIfJobName(constants.GKE_JOBS);
191-
skipIfJobName(constants.AKS_JOBS);
192-
193-
// Skip on PR jobs (presubmit)
194-
skipIfJobType('presubmit');
195-
196-
// Skip on non-OpenShift
197-
skipIfIsOpenShift('false');
198-
```
199-
200-
### Retry Patterns for Flaky Assertions
201-
202-
```typescript
203-
// Use expect().toPass() for inherently async operations
204-
await expect(async () => {
205-
await page.reload();
206-
await expect(page.getByText('entity-name')).toBeVisible();
207-
}).toPass({ intervals: [2000, 5000, 10000], timeout: 60_000 });
208-
```
209-
210-
### Forbidden Patterns
137+
## Coding Conventions
211138

212-
- **Never** use `page.waitForNetworkIdle()` or `networkidle` — it's deprecated and unreliable
213-
- **Never** use raw CSS class selectors (`.MuiButton-root`) — use semantic selectors
214-
- **Avoid** `page.waitForTimeout()` for synchronization — use proper waits
215-
- **Never** hardcode secrets or credentials in test files
139+
Follow the coding conventions defined in the project rules (`playwright-locators`, `ci-e2e-testing`). Key points: use semantic role-based locators, include component annotations in every spec file, never use CSS class selectors or `waitForNetworkIdle()`.
216140

217141
## Cross-Repo Investigation
218142

0 commit comments

Comments
 (0)