Skip to content

Commit 77e259a

Browse files
committed
Unify error handling with getErrorMessage and structured logging, centralize env/host resolution, improve version detection, verify Nextcloud storage success banner, default tests to headless, and document Playwright workflows and test conventions. Added SKILLS.md (Cursor).
1 parent e65b956 commit 77e259a

18 files changed

Lines changed: 680 additions & 140 deletions

.cursor/skills/tests/SKILL.md

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
---
2+
name: tests
3+
description: E2E test conventions for OpenProject integration tests. Use when writing, modifying, or reviewing Playwright tests, page objects, utilities, or locators in this repository.
4+
---
5+
6+
# OpenProject E2E Tests Skill
7+
8+
## Architecture Overview
9+
10+
- `tests/`: Playwright spec files (integration flows across OpenProject, Nextcloud, Keycloak).
11+
- `pageobjects/`: Page Object Model classes wrapping UI interactions.
12+
- `locators/`: JSON locator definitions for each product (`openproject.json`, `nextcloud.json`, `keycloak.json`).
13+
- `utils/`: Shared helpers (config, env/hosts, API clients, error handling, logging, version detection, test helpers).
14+
- `global-setup.ts`: Pre-test setup (Kubernetes setup-job wait, version detection, env var enrichment).
15+
- `playwright.config.ts`: Playwright runner config (headless by default, workers, retries, etc.).
16+
17+
Page object inheritance:
18+
- `BasePage` → domain base pages (`OpenProjectBasePage`, `NextcloudBasePage`, `KeycloakBasePage`) → concrete pages (Login, Home, Admin, etc.).
19+
20+
## Locator Rules
21+
22+
- Never put selectors directly into test code; always use locator rules and JSON files in `locators/`.
23+
- Locator file structure:
24+
25+
```markdown
26+
{
27+
"url": "https://example/",
28+
"selectors": {
29+
"loginButton": { "by": "getByRole", "value": { "role": "button", "name": "Login" } }
30+
}
31+
}
32+
```
33+
34+
- Prefer semantic locators: `getByRole`, `getByLabel`, `getByText`, `getByPlaceholder`, `getByTitle`.
35+
- Use `getByTestId` when `data-testid` attributes are available.
36+
- Use `locator` (CSS/XPath) only as a last resort when semantic selectors are not possible.
37+
- Keep locator keys descriptive and stable (e.g., `usernameInput`, `projectSettingsButton`, `storageCreationSuccessMessage`).
38+
- See `utils/locators_guide.md` for detailed locator patterns and supported `by` values.
39+
40+
## Page Object Conventions
41+
42+
- When adding a new test, first check if an existing page object can be reused or extended before creating a new one.
43+
- All page objects must:
44+
- Extend the appropriate domain base page (`OpenProjectBasePage`, `NextcloudBasePage`, `KeycloakBasePage`) or `BasePage`.
45+
- Load locators via the base constructor using the correct locator JSON file.
46+
- Use `getLocator(key)` to resolve selectors; do not hardcode selectors inside page objects.
47+
- Domain base pages:
48+
- `OpenProjectBasePage` uses `locators/openproject.json`.
49+
- `NextcloudBasePage` uses `locators/nextcloud.json`.
50+
- `KeycloakBasePage` uses `locators/keycloak.json`.
51+
- Export page objects via `index.ts` barrel files per domain for consistent imports.
52+
- Encapsulate complex flows in page methods rather than duplicating sequences in tests.
53+
54+
## Error Handling Standards
55+
56+
- Always use typed catches:
57+
58+
```ts
59+
try {
60+
// ...
61+
} catch (error: unknown) {
62+
logError("Something failed", getErrorMessage(error));
63+
}
64+
```
65+
66+
- Never use `catch (error: any)` or untyped `catch (error)` in new or modified code.
67+
- Use `getErrorMessage(error)` from `utils/error-utils.ts` to safely extract an error message from unknown values.
68+
- Avoid truly silent failures:
69+
- For expected fallbacks, at least log with `logDebug` or `logWarn`.
70+
- For unexpected failures, use `logError` and either rethrow or fail fast depending on context.
71+
72+
## Logging Standards
73+
74+
- Use the central logger from `utils/logger.ts`:
75+
- `logDebug(...)`
76+
- `logInfo(...)`
77+
- `logWarn(...)`
78+
- `logError(...)`
79+
- Do not use `console.log`, `console.warn`, or `console.error` directly in tests, page objects, or utils.
80+
- Log level is controlled by `E2E_LOG_LEVEL` (`debug`, `info`, `warn`, `error`) and defaults to `info`.
81+
- Use `logDebug` for verbose troubleshooting.
82+
- Use `logInfo` for high-level lifecycle information (start/end of major flows).
83+
- Use `logWarn` for recoverable issues and fallbacks.
84+
- Use `logError` for failures that should normally fail the run or require attention.
85+
86+
## Environment and Configuration
87+
88+
- Always resolve environment name and hosts via `utils/env-hosts.ts`:
89+
- `resolveEnvName()` chooses the environment (`local`, `edge`, `stage`, etc.) based on `E2E_ENV` or `ENV` (default: `local`).
90+
- `resolveHosts(envName?)` returns OpenProject, Nextcloud, and Keycloak hosts using env vars with per-env defaults.
91+
- Do not duplicate host or environment resolution logic inside tests or helpers; always call the shared utilities.
92+
- Configuration is loaded once in `utils/config.ts` and exposed as `testConfig`:
93+
- Service URLs and versions.
94+
- Setup method and environment name.
95+
- Any additional test-level configuration.
96+
- `global-setup.ts`:
97+
- Optionally waits for Kubernetes `setup-job` completion when `SETUP_JOB_CHECK=true` (uses `utils/pod-waiter.ts`).
98+
- Runs `detectAllVersions()` from `utils/version-detect.ts` to populate version-related env vars (OpenProject, Nextcloud, Keycloak, etc.) if not already set.
99+
100+
## Type Safety
101+
102+
- Prefer explicit TypeScript interfaces and types for:
103+
- API responses (e.g., OpenProject v3, Nextcloud capabilities and status endpoints, Keycloak server info).
104+
- Config structures and helper return types.
105+
- Avoid `Record<string, any>` and other untyped shapes in new or refactored code.
106+
- Keep interfaces close to their usage (e.g., in `utils/version-detect.ts` for version APIs).
107+
108+
## Test Helpers and Reuse
109+
110+
- Use functions for repetitive actions so they can be reused later rather than copied into each spec.
111+
- Shared helpers live primarily in:
112+
- `utils/test-helpers.ts` for high-level flows (e.g., `ensureProjectExists`, `ensureProjectHasNextcloudStorage`, `ensureDemoProjectCopyViaUi`).
113+
- `utils/openproject-api.ts` for direct API interactions with OpenProject (projects, users, storages).
114+
- When adding new cross-test flows:
115+
- First, see if existing helpers can be extended.
116+
- If needed, create a new helper function with a clear name and parameters.
117+
- After UI actions that should succeed, verify the expected UI feedback:
118+
- Example: after adding Nextcloud storage from OpenProject, wait for the success banner (`storageCreationSuccessMessage` locator, text `"Successful creation."`).
119+
120+
## Running Tests
121+
122+
- Tests run in headless mode by default (see `playwright.config.ts`).
123+
- To run in headed mode, use Playwright’s native CLI flags, e.g.:
124+
- `npx playwright test --headed`
125+
- Default worker configuration:
126+
- Single worker (`workers: 1`) to keep tests predictable and avoid cross-test interference.
127+
- Common commands:
128+
- Run tests: `npx playwright test`
129+
- Run tests headed: `npx playwright test --headed`
130+
- Run tests and open report: `npx playwright test && npx playwright show-report`
131+
132+
## Self-Improvement Directive
133+
134+
- After completing significant refactoring, introducing new patterns, or making architectural changes to this E2E codebase:
135+
- Update this `SKILL.md` to reflect the new conventions, helpers, and standards.
136+
- Keep the file concise and focused on project-specific knowledge that future sessions should follow by default.
137+

.gitignore

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,4 @@ playwright-report/
1313
test-results/
1414
.playwright-browsers/
1515

16-
.DS_Store
17-
.cursor/
16+
.DS_Store

README.md

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -19,55 +19,55 @@ Playwright end-to-end tests for OpenProject-Nextcloud-Keycloak integration.
1919
npm run playwright:install
2020
```
2121

22-
2. **Set env for local (gitignored):**
22+
2. **Ensure local integration cluster is running (for local tests):**
23+
The recommended way to run a full local OpenProject–Nextcloud–Keycloak stack is via the
24+
[`opf/integration-qa-helmfile`](https://github.com/opf/integration-qa-helmfile/tree/main) repo.
25+
Follow its README to `make setup` and `make deploy`, which will provision:
26+
- `https://openproject.test`
27+
- `https://nextcloud.test`
28+
- `https://keycloak.test`
29+
30+
3. **Run tests (native Playwright, headless by default):**
2331
```bash
24-
cat > .env.local <<'EOF'
25-
OPENPROJECT_HOST=openproject.test
26-
NEXTCLOUD_HOST=nextcloud.test
27-
KEYCLOAK_HOST=keycloak.test
28-
E2E_OP_ADMIN_USER=admin
29-
E2E_OP_ADMIN_PASS=admin
30-
E2E_NC_ADMIN_USER=admin
31-
E2E_NC_ADMIN_PASS=admin
32-
E2E_KC_ADMIN_USER=admin
33-
E2E_KC_ADMIN_PASS=admin
34-
EOF
35-
```
36-
37-
3. **Run tests (choose env):**
38-
```bash
39-
E2E_ENV=local npm test # local Helm/defaults
40-
E2E_ENV=edge npm test # edge/staging secrets must be exported
41-
E2E_ENV=stage npm test # stage secrets must be exported
32+
E2E_ENV=local npx playwright test # local Helm/defaults
33+
E2E_ENV=edge npx playwright test # edge/staging secrets must be exported
34+
E2E_ENV=stage npx playwright test # stage secrets must be exported
4235
```
4336

4437
## Commands
4538

4639
### Run Tests
4740
```bash
48-
# Run with explicit env selection (preferred)
49-
E2E_ENV=edge npm test
50-
E2E_ENV=stage npm test
51-
E2E_ENV=local npm test
41+
# Native Playwright (recommended)
42+
E2E_ENV=local npx playwright test
43+
E2E_ENV=edge npx playwright test
44+
E2E_ENV=stage npx playwright test
45+
46+
# Run tests and automatically open HTML report
47+
E2E_ENV=local npx playwright test && npx playwright show-report
5248

5349
# Override worker count to re-enable parallelism (default is 1)
5450
npx playwright test --workers 4
5551

56-
# Shortcuts
52+
# npm script shortcuts (thin wrappers around the above)
5753
npm run test:edge # uses E2E_ENV=edge
5854
npm run test:stage # uses E2E_ENV=stage
5955
npm run test:local # uses E2E_ENV=local
6056

61-
# Run in headed mode
62-
npm run test:e2e:headed
57+
# Run in headed mode (non-headless)
58+
npm run test:e2e:headed
59+
npx playwright test --headed
6360

6461
# Run with UI mode
6562
npm run test:e2e:ui
6663
```
6764

6865
### View Results
6966
```bash
70-
# Open test report
67+
# Open last test report (native)
68+
npx playwright show-report
69+
70+
# Or via npm script
7171
npm run report:show
7272
```
7373

@@ -167,12 +167,11 @@ Local runs: put the above in `.env.local` (already gitignored). CI ignores this
167167

168168
**Playwright complains about missing browser executable?**
169169
- Make sure you've run `npm run playwright:install` (or `npx playwright install`) after `npm install`.
170-
- Re-run your test command, e.g. `npm run test:local`.
171-
```
170+
- Re-run your test command, e.g. `E2E_ENV=local npx playwright test`.
172171

173172
## More Information
174173

175-
- Tests run in parallel by default (both browsers + multiple test files)
174+
- Tests run with a single worker by default (configurable via `--workers`)
176175
- Uses Page Object Model pattern
177176
- Locators stored in JSON files
178177
- See `e2e/utils/locators_guide.md` for locator usage examples

global-setup.ts

Lines changed: 60 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,73 @@
11
import { FullConfig } from '@playwright/test';
22
import { waitForSetupJobComplete, setupJobExists, isSetupJobComplete } from './utils/pod-waiter';
3+
import { detectAllVersions } from './utils/version-detect';
4+
import { getErrorMessage } from './utils/error-utils';
5+
import { logInfo, logError, logWarn } from './utils/logger';
36

47
/**
5-
* Global setup that runs before all tests
6-
* Waits for setup-job to complete only if SETUP_JOB_CHECK=true is set
8+
* Global setup that runs before all tests.
9+
*
10+
* 1. Waits for the setup-job to complete (if SETUP_JOB_CHECK=true).
11+
* 2. Detects service versions via API and stores them as environment
12+
* variables so that config.ts and tests can read them.
13+
* Existing env vars are treated as overrides and are never replaced.
714
*/
815
async function globalSetup(config: FullConfig) {
9-
// Default: skip setup-job check unless explicitly enabled
10-
if (process.env.SETUP_JOB_CHECK !== 'true') {
11-
console.log('⏭️ Skipping setup-job check (enable with SETUP_JOB_CHECK=true)');
12-
return;
13-
}
14-
15-
const namespace = process.env.KUBERNETES_NAMESPACE || 'opnc-integration';
16-
17-
// Check if setup-job exists
18-
const exists = await setupJobExists(namespace);
19-
if (!exists) {
20-
console.log(`⚠️ Setup-job not found in namespace '${namespace}'. Skipping check.`);
21-
console.log(' If you are running tests against a pre-deployed environment,');
22-
console.log(' set SKIP_SETUP_JOB_CHECK=true to skip this check.');
23-
return;
24-
}
16+
// ── Step 1: Setup-job check (optional) ──────────────────────────
17+
if (process.env.SETUP_JOB_CHECK === 'true') {
18+
const namespace = process.env.KUBERNETES_NAMESPACE || 'opnc-integration';
2519

26-
// Check if already complete
27-
const isComplete = await isSetupJobComplete(namespace);
28-
if (isComplete) {
29-
console.log('✓ Setup-job is already completed');
30-
return;
20+
// Check if setup-job exists
21+
const exists = await setupJobExists(namespace);
22+
if (!exists) {
23+
logWarn(`⚠️ Setup-job not found in namespace '${namespace}'. Skipping check.`);
24+
logInfo(' If you are running tests against a pre-deployed environment,');
25+
logInfo(' set SKIP_SETUP_JOB_CHECK=true to skip this check.');
26+
} else {
27+
// Check if already complete
28+
const isComplete = await isSetupJobComplete(namespace);
29+
if (isComplete) {
30+
logInfo('✓ Setup-job is already completed');
31+
} else {
32+
// Wait for setup-job to complete
33+
try {
34+
await waitForSetupJobComplete(namespace);
35+
} catch (error: unknown) {
36+
logError('❌ Setup-job check failed:', getErrorMessage(error));
37+
logError('\nTo skip this check, set SKIP_SETUP_JOB_CHECK=true');
38+
throw error;
39+
}
40+
}
41+
}
42+
} else {
43+
logInfo('⏭️ Skipping setup-job check (enable with SETUP_JOB_CHECK=true)');
3144
}
3245

33-
// Wait for setup-job to complete
46+
// ── Step 2: Detect service versions via API ─────────────────────
3447
try {
35-
await waitForSetupJobComplete(namespace);
36-
} catch (error: any) {
37-
console.error('❌ Setup-job check failed:', error.message);
38-
console.error('\nTo skip this check, set SKIP_SETUP_JOB_CHECK=true');
39-
throw error;
48+
const detected = await detectAllVersions();
49+
50+
// Store detected versions as env vars.
51+
// Existing env vars take precedence (act as manual overrides).
52+
// States like 'not-reachable' and 'not-installed' are preserved
53+
// so tests can see which services/apps are unavailable.
54+
const envMap: Record<string, string> = {
55+
OPENPROJECT_VERSION: detected.openproject,
56+
NEXTCLOUD_VERSION: detected.nextcloud,
57+
NEXTCLOUD_API_VERSION: detected.nextcloudApiVersion,
58+
INTEGRATION_APP_VERSION: detected.integrationApp,
59+
NEXTCLOUD_TEAM_FOLDERS_VERSION: detected.teamFolders,
60+
KEYCLOAK_VERSION: detected.keycloak,
61+
};
62+
63+
for (const [key, value] of Object.entries(envMap)) {
64+
if (!process.env[key]) {
65+
process.env[key] = value;
66+
}
67+
}
68+
} catch (error: unknown) {
69+
logWarn('⚠️ Version detection failed:', getErrorMessage(error));
70+
logWarn(' Tests will use "not-detected" as fallback.');
4071
}
4172
}
4273

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pageobjects/keycloak/KeycloakClientsPage.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Page } from '@playwright/test';
22
import { KeycloakBasePage } from './KeycloakBasePage';
3+
import { getErrorMessage } from '../../utils/error-utils';
34

45
export class KeycloakClientsPage extends KeycloakBasePage {
56
constructor(page: Page) {
@@ -69,8 +70,11 @@ export class KeycloakClientsPage extends KeycloakBasePage {
6970

7071
console.log(`[CLIENTS VERIFICATION] Final result: ${result}`);
7172
return result;
72-
} catch (error) {
73-
console.error('[CLIENTS VERIFICATION] Error verifying clients:', error);
73+
} catch (error: unknown) {
74+
console.error(
75+
'[CLIENTS VERIFICATION] Error verifying clients:',
76+
getErrorMessage(error),
77+
);
7478
// Take a screenshot for debugging
7579
await this.screenshot('clients-verification-error.png').catch(() => {});
7680
return false;

0 commit comments

Comments
 (0)