diff --git a/.claude/agents/playwright-test-generator.md b/.claude/agents/playwright-test-generator.md index 901c90d5e81..2db7f47dd39 100644 --- a/.claude/agents/playwright-test-generator.md +++ b/.claude/agents/playwright-test-generator.md @@ -20,6 +20,16 @@ You are the Playwright Test Generator for the SigNoz frontend. You take a plan w await expect(page.getByRole('tab', { name: /alert rules/i })).toBeVisible(); }); ``` +- **Extended fixtures:** For features needing complex setup (seeded data, API calls, cleanup), import from domain-specific fixtures that extend `auth`. See [docs/contributing/tests/e2e.md](../../docs/contributing/tests/e2e.md) for the full pattern. + - `fixtures/alerts/alert-rules` — worker-scoped rule list + test-scoped rule factory + - `fixtures/alerts/alert-history` — extends alert-rules, adds history fixtures (waits on ruler evaluation) + ```ts + // Alert list tests - need rules, no history + import { test, expect } from '../../../fixtures/alerts/alert-rules'; + + // Alert history tests - need evaluated history rows + import { test, expect } from '../../../fixtures/alerts/alert-history'; + ``` - **Test titles:** `TC-NN ` — matches the planner's IDs. - **Self-contained state.** The bootstrap creates a fresh stack with **zero** dashboards / alerts / etc. — never assume pre-existing data. Two cleanup shapes are valid; pick based on the spec size: - **Per-test `try / finally`** — small specs (~ <10 scenarios) where each test owns its data. diff --git a/.claude/agents/playwright-test-healer.md b/.claude/agents/playwright-test-healer.md index c6a1c49c69d..1ee8755c2f5 100644 --- a/.claude/agents/playwright-test-healer.md +++ b/.claude/agents/playwright-test-healer.md @@ -49,6 +49,7 @@ Don't try to start the stack yourself — it can take ~4 minutes on a cold build - **The list pages render zero-state when the workspace is empty.** Many locators (search input, sort button, `new-dashboard-cta` testid, "All Dashboards" header) are absent in zero-state. A 30s timeout on those usually means the workspace was empty — seed first via `createDashboardViaApi`. - **The "Enter dashboard name…" inline field is a `RequestDashboardBtn` (template-request feedback form), not a create flow.** Tests that try to use it to create a named dashboard will silently no-op. The only UI create paths are the "New dashboard" dropdown → "Create dashboard" (default name "Sample Title", see `DEFAULT_DASHBOARD_TITLE`) or "Import JSON". - **Auth.** `tests/e2e/fixtures/auth.ts` logs in once per worker and caches `storageState` (cookies + localStorage with `AUTH_TOKEN`). For API-driven seeding/cleanup, use `authToken(page)` from `helpers/dashboards.ts` and pass `Authorization: Bearer `. Never re-implement login. +- **Extended fixtures.** Domain-specific fixtures extend `auth` and add seeded data. Alerts uses `fixtures/alerts/alert-rules` (worker-scoped rule list, test-scoped factory) and `fixtures/alerts/alert-history` (extends alert-rules, waits on ruler evaluation). See [docs/contributing/tests/e2e.md](../../docs/contributing/tests/e2e.md) for the pattern. When a test fails on missing data, check if it imports the wrong fixture level. - **Ant Design popovers** (sort menu, action menu) are click-toggle. The trigger element is often an inline `` with a `data-testid` — clicking it opens the popover; clicking it again closes. After selecting an option, the popover auto-closes. If a test interacts with the popover twice, wait for the menu items to be visible explicitly between toggles. - **Artifacts.** Every failed test writes to `tests/e2e/artifacts/results//` — the `error-context.md` accessibility snapshot is the fastest way to see what the page actually looked like when it failed. - **Type-check.** After edits, run `npx tsc --noEmit -p tests/e2e/tsconfig.json` if it succeeds, or rely on `npx playwright test --list` to validate the spec parses. diff --git a/docs/contributing/tests/e2e.md b/docs/contributing/tests/e2e.md index acac912114f..164db0f3147 100644 --- a/docs/contributing/tests/e2e.md +++ b/docs/contributing/tests/e2e.md @@ -112,6 +112,41 @@ These two folders look similar but mean different things: Rule of thumb: if it's a `test.extend` fixture, put it in `fixtures/`. If it's a function you call explicitly (or a constant the function uses), put it in `helpers/`. If it's a static file the helpers read, put it in `testdata/`. +### Extended fixtures + +For features needing complex setup (API-seeded data, ruler evaluation waits, cleanup), create domain-specific fixtures that extend `auth`. Group them in `fixtures//`. + +**Fixture scopes:** +- **test scope** — fresh data per test. Use for mutations (edit, delete, rename). +- **worker scope** — shared across tests in one worker. Use for read-only data. Worker scope pays the setup cost once per worker instead of once per test. + +**The alerts pattern** (`fixtures/alerts/`) demonstrates extending fixtures: + +``` +fixtures/alerts/ +├── alert-rules.ts # extends auth — worker-scoped rule list + test-scoped factory +└── alert-history.ts # extends alert-rules — adds history fixtures (waits on ruler) +``` + +Specs import from the fixture they need: + +```ts +// List tests — just need rules, no history +import { test, expect } from '../../../fixtures/alerts/alert-rules'; + +// History tests — need history rows from ruler evaluation +import { test, expect } from '../../../fixtures/alerts/alert-history'; +``` + +**When creating new fixtures:** + +1. **Identify scope** — Will tests mutate the data? If yes, test-scoped. If read-only, worker-scoped. +2. **Group by domain** — Put fixtures in `fixtures//`. Helpers in `helpers//`. +3. **Extend existing fixtures** — Chain from `auth` or another fixture to inherit its setup. +4. **Handle timeouts** — Worker-scoped fixtures that wait on backend processing need explicit timeouts. +5. **Clean up** — Always delete seeded data in the fixture teardown (after `use()`). +6. **Extract logic into functions** — Keep the `test.extend()` block lean; move setup/teardown logic to named functions so the extend block reads as a manifest of "what fixtures exist." + Each spec follows these principles: 1. **Directory per feature**: `tests/e2e/tests//*.spec.ts`. Cross-resource junction concerns (e.g. cascade-delete) go in their own file, not packed into one giant spec. @@ -232,11 +267,14 @@ cd tests/e2e # Single feature dir npx playwright test tests/alerts/ --project=chromium +# Single sub-area +npx playwright test tests/alerts/history/ --project=chromium + # Single file -npx playwright test tests/alerts/alerts.spec.ts --project=chromium +npx playwright test tests/alerts/page.spec.ts --project=chromium # Single test by title grep -npx playwright test --project=chromium -g "TC-01" +npx playwright test --project=chromium -g "AL-01" ``` ### Iterative modes @@ -270,7 +308,14 @@ yarn test:staging | `SIGNOZ_E2E_PASSWORD` | Admin password. Bootstrap writes the integration-test default. | | `SIGNOZ_E2E_SEEDER_URL` | Seeder HTTP base URL — hit by specs that need per-test telemetry. | -Loading order in `playwright.config.ts`: `.env` first (user-provided, staging), then `.env.local` with `override: true` (bootstrap-generated, local mode). Anything already set in `process.env` at yarn-test time wins because dotenv doesn't touch vars that are already present. +Precedence in `playwright.config.ts`, lowest to highest: `.env` (user-provided, staging) → `.env.local` (bootstrap-generated, local mode) → whatever is already in `process.env`. The config parses both files itself and only fills in keys the environment does not already define, so exporting a variable always wins: + +```bash +# runs against a locally served frontend, not whatever .env.local points at +SIGNOZ_E2E_BASE_URL=http://127.0.0.1:3301 pnpm test tests/alerts +``` + +This is deliberately not `dotenv.config({ override: true })`. That flag makes the *file* beat `process.env`, which silently discarded exported values — including the `SIGNOZ_E2E_BASE_URL` in `pnpm test:staging`, whenever a `.env.local` happened to exist. ### Playwright options diff --git a/frontend/src/api/ErrorResponseHandler.ts b/frontend/src/api/ErrorResponseHandler.ts index 6d972ec90f4..573a665de21 100644 --- a/frontend/src/api/ErrorResponseHandler.ts +++ b/frontend/src/api/ErrorResponseHandler.ts @@ -53,7 +53,7 @@ export function ErrorResponseHandler(error: AxiosError): ErrorResponse { }; } // anything else - console.error('any'); + console.error('ErrorResponseHandler: unclassified error'); return { statusCode: 500, payload: null, diff --git a/frontend/src/components/AlertBreadcrumb/AlertBreadcrumb.tsx b/frontend/src/components/AlertBreadcrumb/AlertBreadcrumb.tsx index e56dba0b0ad..49ab5fc37b7 100644 --- a/frontend/src/components/AlertBreadcrumb/AlertBreadcrumb.tsx +++ b/frontend/src/components/AlertBreadcrumb/AlertBreadcrumb.tsx @@ -8,12 +8,14 @@ export interface AlertBreadcrumbProps { items: BreadcrumbItemConfig[]; className?: string; showDivider?: boolean; + testId?: string; } function AlertBreadcrumb({ items, className, showDivider = true, + testId, }: AlertBreadcrumbProps): JSX.Element { const breadcrumbItems = items.map((item) => ({ title: , @@ -24,6 +26,7 @@ function AlertBreadcrumb({ {showDivider && } diff --git a/frontend/src/components/FieldsSelector/FieldsSelector.tsx b/frontend/src/components/FieldsSelector/FieldsSelector.tsx index da1947d79e3..09506096d7f 100644 --- a/frontend/src/components/FieldsSelector/FieldsSelector.tsx +++ b/frontend/src/components/FieldsSelector/FieldsSelector.tsx @@ -197,7 +197,7 @@ function FieldsSelector({ () => fields.map((f) => ({ ...f, - key: buildCompositeKey(f.name, f.fieldContext), + key: buildCompositeKey(f.name, f.fieldContext, f.fieldDataType), })), [fields], ); diff --git a/frontend/src/components/FieldsSelector/OtherFields.tsx b/frontend/src/components/FieldsSelector/OtherFields.tsx index 6f9a3a5dddb..14beb40fc0d 100644 --- a/frontend/src/components/FieldsSelector/OtherFields.tsx +++ b/frontend/src/components/FieldsSelector/OtherFields.tsx @@ -52,13 +52,15 @@ function OtherFields({ // Normalize: synthesize `key` once so downstream reads can trust it. const suggestions: TelemetryFieldKey[] = rawSuggestions.map((attr) => ({ ...attr, - key: buildCompositeKey(attr.name, attr.fieldContext as string), + key: buildCompositeKey(attr.name, attr.fieldContext, attr.fieldDataType), signal: attr.signal as SignalType, fieldContext: attr.fieldContext as FieldContext, fieldDataType: attr.fieldDataType, })); const addedIds = new Set( - addedFields.map((f) => buildCompositeKey(f.name, f.fieldContext)), + addedFields.map((f) => + buildCompositeKey(f.name, f.fieldContext, f.fieldDataType), + ), ); const available = suggestions.filter( (attr) => !addedIds.has(attr.key as string), diff --git a/frontend/src/components/Logs/TableView/__tests__/useLogsTableColumns.test.tsx b/frontend/src/components/Logs/TableView/__tests__/useLogsTableColumns.test.tsx index 09ce26ad2b1..0942cf37e21 100644 --- a/frontend/src/components/Logs/TableView/__tests__/useLogsTableColumns.test.tsx +++ b/frontend/src/components/Logs/TableView/__tests__/useLogsTableColumns.test.tsx @@ -14,10 +14,10 @@ jest.mock('providers/App/App', () => ({ useAppContext: (): { featureFlags: [] } => ({ featureFlags: [] }), })); -const field = (name: string, type = ''): IField => ({ +const field = (name: string, type = '', dataType = ''): IField => ({ name, type, - dataType: 'string', + dataType, }); describe('useLogsTableColumns — selectColumns-order respected', () => { @@ -136,6 +136,24 @@ describe('useLogsTableColumns — selectColumns-order respected', () => { expect(byId.get('user_field')?.enableRemove).toBe(true); }); + it('disambiguates same-name/same-context fields by dataType (3-part id)', () => { + const { result } = renderHook(() => + useLogsTableColumns({ + fields: [ + field('http.status_code', 'attribute', 'int64'), + field('http.status_code', 'attribute', 'string'), + ], + fontSize: FontSize.SMALL, + }), + ); + + expect(result.current.map((c) => c.id)).toStrictEqual([ + 'state-indicator', + 'attribute:http.status_code:int64', + 'attribute:http.status_code:string', + ]); + }); + it('renders only the stateIndicator when fields is empty', () => { const { result } = renderHook(() => useLogsTableColumns({ diff --git a/frontend/src/components/Logs/TableView/useLogsTableColumns.tsx b/frontend/src/components/Logs/TableView/useLogsTableColumns.tsx index f3a52bb0fd8..8116556d3b9 100644 --- a/frontend/src/components/Logs/TableView/useLogsTableColumns.tsx +++ b/frontend/src/components/Logs/TableView/useLogsTableColumns.tsx @@ -92,7 +92,7 @@ export function useLogsTableColumns({ }; const makeUserFieldCol = (f: IField): TableColumnDef => ({ - id: buildCompositeKey(f.name, f.type), + id: buildCompositeKey(f.name, f.type, f.dataType), header: f.name, accessorFn: (log): unknown => getLogFieldValue(log, f.name, isBodyJsonEnabled), diff --git a/frontend/src/container/AlertHistory/AlertPopover/AlertPopover.tsx b/frontend/src/container/AlertHistory/AlertPopover/AlertPopover.tsx index 4211a6b57bb..913de2cd6d8 100644 --- a/frontend/src/container/AlertHistory/AlertPopover/AlertPopover.tsx +++ b/frontend/src/container/AlertHistory/AlertPopover/AlertPopover.tsx @@ -29,6 +29,7 @@ function PopoverContent({
@@ -40,6 +41,7 @@ function PopoverContent({
0) { return ( -
+
@@ -38,7 +41,10 @@ function ChangePercentage({ } if (direction < 0) { return ( -
+
@@ -50,7 +56,10 @@ function ChangePercentage({ } return ( -
+
no previous data
); @@ -103,7 +112,12 @@ function StatsCard({ const formattedEndTimeForTooltip = convertTimestampToLocaleDateString(endTime); return ( -
+
{title}
@@ -123,7 +137,7 @@ function StatsCard({
-
+
{isEmpty ? emptyMessage : displayValue || totalCurrentCount}
diff --git a/frontend/src/container/AlertHistory/Statistics/StatsCard/StatsGraph/StatsGraph.tsx b/frontend/src/container/AlertHistory/Statistics/StatsCard/StatsGraph/StatsGraph.tsx index 041160bd598..4b1cf4b5772 100644 --- a/frontend/src/container/AlertHistory/Statistics/StatsCard/StatsGraph/StatsGraph.tsx +++ b/frontend/src/container/AlertHistory/Statistics/StatsCard/StatsGraph/StatsGraph.tsx @@ -81,7 +81,11 @@ function StatsGraph({ timeSeries, changeDirection }: Props): JSX.Element { ); return ( -
+
); diff --git a/frontend/src/container/AlertHistory/Statistics/TopContributorsCard/TopContributorsCard.tsx b/frontend/src/container/AlertHistory/Statistics/TopContributorsCard/TopContributorsCard.tsx index a1d4f293186..ae0054e48e2 100644 --- a/frontend/src/container/AlertHistory/Statistics/TopContributorsCard/TopContributorsCard.tsx +++ b/frontend/src/container/AlertHistory/Statistics/TopContributorsCard/TopContributorsCard.tsx @@ -48,11 +48,16 @@ function TopContributorsCard({ return ( <> -
+
top contributors
{topContributorsData.length > 3 && ( - @@ -65,7 +64,7 @@ export const timelineTableColumns = ({ relatedTracesLink={record.relatedTracesLink ?? ''} relatedLogsLink={record.relatedLogsLink ?? ''} > - diff --git a/frontend/src/container/AlertHistory/Timeline/TabsAndFilters/TabsAndFilters.tsx b/frontend/src/container/AlertHistory/Timeline/TabsAndFilters/TabsAndFilters.tsx index c23a46a4c1c..5f09a8a3b35 100644 --- a/frontend/src/container/AlertHistory/Timeline/TabsAndFilters/TabsAndFilters.tsx +++ b/frontend/src/container/AlertHistory/Timeline/TabsAndFilters/TabsAndFilters.tsx @@ -23,6 +23,7 @@ function TimelineTabs(): JSX.Element { { value: TimelineTab.OVERALL_STATUS, label: 'Overall Status', + testId: 'timeline-tab-overall-status', }, { value: TimelineTab.TOP_5_CONTRIBUTORS, @@ -33,6 +34,7 @@ function TimelineTabs(): JSX.Element {
), disabled: true, + testId: 'timeline-tab-top-contributors', }, ]; @@ -57,14 +59,17 @@ function TimelineFilters(): JSX.Element { { value: TimelineFilter.ALL, label: 'All', + testId: 'timeline-filter-all', }, { value: TimelineFilter.FIRED, label: 'Fired', + testId: 'timeline-filter-fired', }, { value: TimelineFilter.RESOLVED, label: 'Resolved', + testId: 'timeline-filter-resolved', }, ]; diff --git a/frontend/src/container/CreateAlertV2/EvaluationSettings/AdvancedOptions.tsx b/frontend/src/container/CreateAlertV2/EvaluationSettings/AdvancedOptions.tsx index f1a2894cbf0..4dbfe201e81 100644 --- a/frontend/src/container/CreateAlertV2/EvaluationSettings/AdvancedOptions.tsx +++ b/frontend/src/container/CreateAlertV2/EvaluationSettings/AdvancedOptions.tsx @@ -34,6 +34,7 @@ function AdvancedOptions(): JSX.Element { }) } value={advancedOptions.sendNotificationIfDataIsMissing.toleranceLimit} + testId="send-notification-if-data-is-missing-input" /> Minutes
@@ -66,6 +67,7 @@ function AdvancedOptions(): JSX.Element { }) } value={advancedOptions.enforceMinimumDatapoints.minimumDatapoints} + testId="enforce-minimum-datapoints-input" /> Datapoints
diff --git a/frontend/src/container/CreateAlertV2/EvaluationSettings/EvaluationWindowPopover/EvaluationWindowPopover.tsx b/frontend/src/container/CreateAlertV2/EvaluationSettings/EvaluationWindowPopover/EvaluationWindowPopover.tsx index bbaa1a8373c..a9aa5d28ca8 100644 --- a/frontend/src/container/CreateAlertV2/EvaluationSettings/EvaluationWindowPopover/EvaluationWindowPopover.tsx +++ b/frontend/src/container/CreateAlertV2/EvaluationSettings/EvaluationWindowPopover/EvaluationWindowPopover.tsx @@ -66,6 +66,7 @@ function EvaluationWindowPopover({ tabIndex={0} data-value={option.value} data-section-id={sectionId} + data-testid={`${sectionId}-option-${option.value}`} onClick={(): void => onChange(option.value)} onKeyDown={(e): void => { if (e.key === 'Enter' || e.key === ' ') { diff --git a/frontend/src/container/CreateAlertV2/Footer/Footer.tsx b/frontend/src/container/CreateAlertV2/Footer/Footer.tsx index 07a9cabbb53..5ee3dca4663 100644 --- a/frontend/src/container/CreateAlertV2/Footer/Footer.tsx +++ b/frontend/src/container/CreateAlertV2/Footer/Footer.tsx @@ -186,6 +186,7 @@ function Footer(): JSX.Element { color="primary" onClick={handleSaveAlert} disabled={disableButtons || Boolean(alertValidationMessage)} + testId="save-alert-rule-button" > {isCreatingAlertRule || isUpdatingAlertRule ? ( @@ -218,6 +219,7 @@ function Footer(): JSX.Element { color="secondary" onClick={handleTestNotification} disabled={disableButtons || Boolean(alertValidationMessage)} + testId="test-notification-button" > {isTestingAlertRule ? ( @@ -249,6 +251,7 @@ function Footer(): JSX.Element { color="secondary" onClick={handleDiscard} disabled={disableButtons} + testId="discard-alert-rule-button" > Discard diff --git a/frontend/src/container/FormAlertRules/BasicInfo.tsx b/frontend/src/container/FormAlertRules/BasicInfo.tsx index cb45a9cd74a..406ff6eadc4 100644 --- a/frontend/src/container/FormAlertRules/BasicInfo.tsx +++ b/frontend/src/container/FormAlertRules/BasicInfo.tsx @@ -119,6 +119,7 @@ function BasicInfo({ { const s = (value as string) || 'critical'; setAlertDef({ @@ -147,6 +148,7 @@ function BasicInfo({ ]} > { setAlertDef({ ...alertDef, @@ -161,6 +163,7 @@ function BasicInfo({ name={['annotations', 'description']} > { setAlertDef({ ...alertDef, diff --git a/frontend/src/container/FormAlertRules/QuerySection.tsx b/frontend/src/container/FormAlertRules/QuerySection.tsx index 111567ed8fb..e53ee7896e0 100644 --- a/frontend/src/container/FormAlertRules/QuerySection.tsx +++ b/frontend/src/container/FormAlertRules/QuerySection.tsx @@ -105,7 +105,7 @@ function QuerySection({ { label: ( - @@ -122,7 +122,11 @@ function QuerySection({ : 'ClickHouse' } > - @@ -162,7 +166,11 @@ function QuerySection({ : 'ClickHouse' } > - @@ -180,7 +188,11 @@ function QuerySection({ : 'PromQL' } > -