diff --git a/.agents/skills/react-generic-components/SKILL.md b/.agents/skills/react-generic-components/SKILL.md new file mode 100644 index 000000000..866c0c046 --- /dev/null +++ b/.agents/skills/react-generic-components/SKILL.md @@ -0,0 +1,278 @@ +--- +name: react-generic-components +description: Guidelines for building truly generic and reusable React components without business logic or specific use-case code in React/TypeScript codebases +license: MIT +allowed-tools: + - read + - write + - edit + - grep +metadata: + version: "1.0" + author: "KernelCI Dashboard Team" + tags: ["react", "components", "architecture", "reusability"] +--- + +# React Generic Components Skill + +This skill ensures React components remain truly generic and reusable by avoiding hardcoded business logic or use-case-specific code. + +## Core Principle + +**Generic components should not contain specific logic for any particular use case.** + +Components should accept configuration through: +- Props +- Configuration objects +- Context (when appropriate) +- Composition patterns + +## Anti-Patterns to Avoid + +### ❌ Hardcoded Business Logic + +```typescript +// ❌ BAD - Checking for specific values in generic component +const TableCell = ({ cell }) => { + const dataTestId = cell.column.id === 'details' + ? 'details-button' + : undefined; + + return ...; +}; +``` + +**Problem**: This component knows about a specific column ID ("details"). It's no longer generic. + +### ❌ Conditional Rendering Based on Specific Values + +```typescript +// ❌ BAD - Special cases for specific data +const StatusBadge = ({ status }) => { + if (status === 'COMPLETED') { + return Done!; + } + if (status === 'FAILED') { + return Error!; + } + return {status}; +}; +``` + +**Problem**: Every new status requires modifying the component. Not scalable. + +### ❌ Mixed Concerns + +```typescript +// ❌ BAD - Component handles both rendering and business logic +const ChartLegend = ({ data }) => { + // Business logic mixed in + if (data.value === 0 && data.label !== 'important') { + return null; + } + + return
...
; +}; +``` + +**Problem**: Generic chart component shouldn't decide what to hide based on business rules. + +## Correct Patterns + +### ✅ Configuration Through Props + +```typescript +// ✅ GOOD - Generic, accepts configuration +interface TableCellProps { + children: React.ReactNode; + dataTestId?: string; + linkProps?: LinkProps; +} + +const TableCell = ({ children, dataTestId, linkProps }: TableCellProps) => { + return ( + + + {children} + + + ); +}; +``` + +**Usage**: +```typescript +// Configuration happens at usage site + + {content} + +``` + +### ✅ Configuration Through Metadata + +```typescript +// ✅ GOOD - Read configuration from metadata +const TableCell = ({ cell }) => { + // Generic: reads from column's configuration + const metaResult = ColumnMetaSchema.safeParse(cell.column.columnDef.meta); + const dataTestId = metaResult.success ? metaResult.data.dataTestId : undefined; + + return ...; +}; +``` + +**Configuration** (separate from component): +```typescript +// Column definition includes metadata +const columns = [ + { + id: 'details', + header: () => , + cell: () => , + meta: { + dataTestId: 'details-button', // ← Configuration + }, + }, +]; +``` + +### ✅ Mapping Objects for Status/Variants + +```typescript +// ✅ GOOD - Configuration-driven +interface StatusBadgeProps { + status: string; + colorMap: Record; + labelMap?: Record; +} + +const StatusBadge = ({ status, colorMap, labelMap }: StatusBadgeProps) => { + const color = colorMap[status] || 'gray'; + const label = labelMap?.[status] || status; + + return {label}; +}; +``` + +**Usage**: +```typescript +const STATUS_COLORS = { + COMPLETED: 'green', + FAILED: 'red', + PENDING: 'yellow', +}; + + +``` + +### ✅ Generic with Label-Based IDs + +```typescript +// ✅ GOOD - Uses generic property from data +const ChartLegend = ({ chartValues, onClick }) => { + return chartValues.map(value => ( + + )); +}; +``` + +## Real-World Example from Codebase + +### Before (Specific Logic in Generic Component) + +```typescript +// ❌ BAD +const TableCellComponent = ({ cell }) => { + const dataTestId = + cell.column.id === DETAILS_COLUMN_ID ? 'details-button' : undefined; + + return ...; +}; +``` + +### After (Generic Configuration-Based) + +```typescript +// ✅ GOOD - Generic component +const ColumnMetaSchema = z.object({ + dataTestId: z.string().optional(), +}); + +const TableCellComponent = ({ cell }) => { + const metaResult = ColumnMetaSchema.safeParse(cell.column.columnDef.meta); + const dataTestId = metaResult.success ? metaResult.data.dataTestId : undefined; + + return ...; +}; + +// Configuration (separate from component) +const buildColumns = [ + { + id: DETAILS_COLUMN_ID, + header: () => , + cell: () => , + meta: { dataTestId: 'details-button' }, + }, +]; +``` + +## Decision Tree for AI Agents + +When adding functionality to a component: + +``` +Is this component used in multiple places? +├─ Yes → Make it generic +│ └─ Can the behavior be configured? +│ ├─ Yes → Use props/metadata +│ └─ No → Consider composition or separate components +└─ No → Specific logic may be acceptable +``` + +## Instructions for AI Agents + +1. **Before modifying a component**: + - Check if it's used in multiple places (`grep` or search) + - Look for patterns like "Generic", "Reusable", or "Common" in file paths + - Check if it's in a `components/ui/` or similar directory + +2. **If the component is generic**: + - Do NOT add hardcoded checks for specific values + - Do NOT add business logic + - Instead, add a prop or use configuration + +3. **Choose the right approach**: + - Simple cases → Add a prop + - Complex cases → Use metadata/configuration objects + - When you need the existing data property → Use that (like `label`, `id`, etc.) + +4. **Verify your solution**: + - Can this component work for ANY use case, not just the current one? + - Is the configuration separate from the component? + - Would adding a new variant require changing the component? + - If yes → Refactor to be more generic + +## Benefits + +- **Reusability**: One component, infinite use cases +- **Maintainability**: Changes happen in configuration, not component code +- **Testability**: Generic components are easier to test +- **Scalability**: New features don't require component changes +- **Type Safety**: Configuration can be validated with Zod + +## References + +- Real implementation: `src/components/Table/TableComponents.tsx` +- Configuration examples: `src/components/BuildsTable/DefaultBuildsColumns.tsx` +- Related skill: `type-safety` (for validating configuration) diff --git a/.agents/skills/typescript-type-safety/SKILL.md b/.agents/skills/typescript-type-safety/SKILL.md new file mode 100644 index 000000000..e32890d45 --- /dev/null +++ b/.agents/skills/typescript-type-safety/SKILL.md @@ -0,0 +1,172 @@ +--- +name: typescript-type-safety +description: Ensures type-safe TypeScript code practices by preferring Zod validation or type guards over type assertions in TypeScript/JavaScript codebases +license: MIT +allowed-tools: + - read + - write + - edit + - bash +metadata: + version: "1.0" + author: "KernelCI Dashboard Team" + tags: ["typescript", "type-safety", "validation"] +--- + +# TypeScript Type Safety Skill + +This skill ensures type-safe TypeScript code practices by enforcing the use of runtime validation (Zod) or type guards instead of type assertions. + +## Core Principles + +### ❌ Avoid Type Assertions + +Type assertions bypass TypeScript's type system and provide no runtime safety: + +```typescript +// ❌ BAD - No runtime validation +const user = unknownData as User; +const id = obj.meta?.id as string | undefined; +``` + +### ✅ Prefer Zod Validation + +Zod provides runtime validation and automatic TypeScript type inference: + +```typescript +// ✅ GOOD - Runtime validation with Zod +import { z } from 'zod'; + +const UserSchema = z.object({ + id: z.string(), + name: z.string(), + age: z.number().optional(), +}); + +const result = UserSchema.safeParse(unknownData); +if (result.success) { + const user = result.data; // Type-safe + // use user... +} else { + // Handle validation error + console.error(result.error); +} +``` + +### ✅ Alternative: Type Guards + +When Zod is not suitable, use type guard functions: + +```typescript +// ✅ GOOD - Type guard with runtime check +function isUser(obj: unknown): obj is User { + return ( + typeof obj === 'object' && + obj !== null && + 'id' in obj && + typeof obj.id === 'string' && + 'name' in obj && + typeof obj.name === 'string' + ); +} + +if (isUser(unknownData)) { + const user = unknownData; // Type-safe + // use user... +} +``` + +## When to Use Each Approach + +### Use Zod When: +- Validating external data (API responses, user input, file contents) +- Working with complex nested objects +- You need detailed error messages +- The shape of data might change over time + +### Use Type Guards When: +- Simple type checks are sufficient +- Performance is critical (type guards are faster than Zod) +- Validating internal data structures +- Checking for specific object shapes + +### Type Assertions Are Acceptable Only When: +- Working with DOM APIs where the type is guaranteed (e.g., `event.target as HTMLInputElement`) +- Third-party library types that are provably correct +- As a last resort with clear documentation explaining why it's safe + +## Real-World Example from Codebase + +From `src/components/Table/TableComponents.tsx`: + +```typescript +// Define schema for column metadata +const ColumnMetaSchema = z.object({ + dataTestId: z.string().optional(), +}); + +// Validate at runtime +const metaResult = ColumnMetaSchema.safeParse(cell.column.columnDef.meta); +const dataTestId = metaResult.success ? metaResult.data.dataTestId : undefined; + +// Use the validated data + + {content} + +``` + +## Instructions for AI Agents + +When you encounter code that needs type safety: + +1. **Identify the source of uncertainty**: Is the data from an external source, user input, or internal code? + +2. **Choose the appropriate approach**: + - For external/untrusted data → Use Zod + - For simple internal checks → Use type guards + - Never use type assertions unless absolutely necessary + +3. **Implement Zod validation**: + ```typescript + import { z } from 'zod'; + + // Define schema + const Schema = z.object({ + // ... your schema + }); + + // Validate + const result = Schema.safeParse(data); + if (result.success) { + // Use result.data + } else { + // Handle error + } + ``` + +4. **Or implement a type guard**: + ```typescript + function isType(obj: unknown): obj is Type { + return ( + typeof obj === 'object' && + obj !== null && + // ... your checks + ); + } + ``` + +5. **Test the implementation**: Ensure your validation handles edge cases and provides helpful error messages + +## Benefits + +- **Runtime Safety**: Catch type mismatches before they cause bugs +- **Better Error Messages**: Zod provides detailed validation errors +- **Self-Documenting**: Schemas serve as documentation +- **Refactoring Safety**: Changes to types are caught by validation +- **Production Confidence**: Validate data at boundaries (API, user input) + +## References + +- Zod Documentation: https://zod.dev +- TypeScript Type Guards: https://www.typescriptlang.org/docs/handbook/2/narrowing.html#using-type-predicates +- Example Implementation: `src/components/Table/TableComponents.tsx` diff --git a/dashboard/e2e/selectors/common.ts b/dashboard/e2e/selectors/common.ts new file mode 100644 index 000000000..434c8a22f --- /dev/null +++ b/dashboard/e2e/selectors/common.ts @@ -0,0 +1,31 @@ +export const COMMON_SELECTORS = { + tableRow: 'tr', + tableHeader: 'th', + + originDropdown: '[data-test-id="origin-dropdown"]', + originOption: (origin: string) => `[data-test-id="origin-option-${origin}"]`, + + STATUS_FILTER_TEXT: { + ALL: 'All:', + SUCCESS: 'Success:', + FAILED: 'Failed:', + INCONCLUSIVE: 'Inconclusive:', + }, + statusFilterSelector: (status: string) => `button:has-text("${status}")`, + + STATUS_LABEL_TEXT: { + PASS: 'PASS', + FAIL: 'FAIL', + ERROR: 'ERROR', + MISS: 'MISS', + SKIP: 'SKIP', + DONE: 'DONE', + NULL: 'NULL', + }, + SECTION_TEXT: { + BUILD_STATUS: 'Build Status', + BOOT_STATUS: 'Boot Status', + }, + statusLabelSelector: (section: string, status: string) => + `text="${section}" >> .. >> label:has-text("${status}")`, +} as const; diff --git a/dashboard/e2e/selectors/index.ts b/dashboard/e2e/selectors/index.ts new file mode 100644 index 000000000..7b3786aa1 --- /dev/null +++ b/dashboard/e2e/selectors/index.ts @@ -0,0 +1,3 @@ +export * from './common'; +export * from './tree-listing'; +export * from './tree-details'; diff --git a/dashboard/e2e/selectors/tree-details.ts b/dashboard/e2e/selectors/tree-details.ts new file mode 100644 index 000000000..68aa5caf6 --- /dev/null +++ b/dashboard/e2e/selectors/tree-details.ts @@ -0,0 +1,165 @@ +import { COMMON_SELECTORS } from './common'; + +export const TREE_DETAILS_SELECTORS = { + breadcrumbTreesLink: '[data-test-id="breadcrumb-trees-link"]', + + treeHeaderTable: 'table', + + tabs: { + builds: 'button:has-text("Builds")', + boots: 'button:has-text("Boots")', + tests: 'button:has-text("Tests")', + }, + + filters: { + button: 'button:has-text("Filters")', + drawer: 'aside', + drawerContent: '[role="dialog"]', + filterButton: '[data-test-id="filter-button"]', + cancelButton: '[data-test-id="filter-cancel-button"]', + clearAllFilters: 'text="Clear all"', + buildStatusSection: `text="${COMMON_SELECTORS.SECTION_TEXT.BUILD_STATUS}"`, + bootStatusSection: `text="${COMMON_SELECTORS.SECTION_TEXT.BOOT_STATUS}"`, + buildStatus: { + pass: COMMON_SELECTORS.statusLabelSelector( + COMMON_SELECTORS.SECTION_TEXT.BUILD_STATUS, + COMMON_SELECTORS.STATUS_LABEL_TEXT.PASS, + ), + fail: COMMON_SELECTORS.statusLabelSelector( + COMMON_SELECTORS.SECTION_TEXT.BUILD_STATUS, + COMMON_SELECTORS.STATUS_LABEL_TEXT.FAIL, + ), + error: COMMON_SELECTORS.statusLabelSelector( + COMMON_SELECTORS.SECTION_TEXT.BUILD_STATUS, + COMMON_SELECTORS.STATUS_LABEL_TEXT.ERROR, + ), + miss: COMMON_SELECTORS.statusLabelSelector( + COMMON_SELECTORS.SECTION_TEXT.BUILD_STATUS, + COMMON_SELECTORS.STATUS_LABEL_TEXT.MISS, + ), + skip: COMMON_SELECTORS.statusLabelSelector( + COMMON_SELECTORS.SECTION_TEXT.BUILD_STATUS, + COMMON_SELECTORS.STATUS_LABEL_TEXT.SKIP, + ), + done: COMMON_SELECTORS.statusLabelSelector( + COMMON_SELECTORS.SECTION_TEXT.BUILD_STATUS, + COMMON_SELECTORS.STATUS_LABEL_TEXT.DONE, + ), + null: COMMON_SELECTORS.statusLabelSelector( + COMMON_SELECTORS.SECTION_TEXT.BUILD_STATUS, + COMMON_SELECTORS.STATUS_LABEL_TEXT.NULL, + ), + }, + bootStatus: { + pass: COMMON_SELECTORS.statusLabelSelector( + COMMON_SELECTORS.SECTION_TEXT.BOOT_STATUS, + COMMON_SELECTORS.STATUS_LABEL_TEXT.PASS, + ), + fail: COMMON_SELECTORS.statusLabelSelector( + COMMON_SELECTORS.SECTION_TEXT.BOOT_STATUS, + COMMON_SELECTORS.STATUS_LABEL_TEXT.FAIL, + ), + error: COMMON_SELECTORS.statusLabelSelector( + COMMON_SELECTORS.SECTION_TEXT.BOOT_STATUS, + COMMON_SELECTORS.STATUS_LABEL_TEXT.ERROR, + ), + miss: COMMON_SELECTORS.statusLabelSelector( + COMMON_SELECTORS.SECTION_TEXT.BOOT_STATUS, + COMMON_SELECTORS.STATUS_LABEL_TEXT.MISS, + ), + skip: COMMON_SELECTORS.statusLabelSelector( + COMMON_SELECTORS.SECTION_TEXT.BOOT_STATUS, + COMMON_SELECTORS.STATUS_LABEL_TEXT.SKIP, + ), + done: COMMON_SELECTORS.statusLabelSelector( + COMMON_SELECTORS.SECTION_TEXT.BOOT_STATUS, + COMMON_SELECTORS.STATUS_LABEL_TEXT.DONE, + ), + }, + }, + + buildHistoryGraph: 'img', + + statusCard: { + title: '.flex-col:has(div:has-text("Build status"))', + titleFirst: '.flex-col:has(div:has-text("Build status"))', + statusButton: (status: string) => `[data-test-id="${status}"]`, + }, + + summaryCards: { + arch: 'text="Summary"', + }, + + issuesCard: { + title: 'text="Issues"', + button: 'button[aria-label="Issues"]', + }, + + buildTable: { + table: 'table', + statusFilters: { + all: COMMON_SELECTORS.statusFilterSelector( + COMMON_SELECTORS.STATUS_FILTER_TEXT.ALL, + ), + success: COMMON_SELECTORS.statusFilterSelector( + COMMON_SELECTORS.STATUS_FILTER_TEXT.SUCCESS, + ), + failed: COMMON_SELECTORS.statusFilterSelector( + COMMON_SELECTORS.STATUS_FILTER_TEXT.FAILED, + ), + inconclusive: COMMON_SELECTORS.statusFilterSelector( + COMMON_SELECTORS.STATUS_FILTER_TEXT.INCONCLUSIVE, + ), + }, + searchInput: 'input[placeholder*="Search"]', + detailsButton: '[data-test-id="details-button"]', + }, + + bootsTable: { + statusFilters: { + all: COMMON_SELECTORS.statusFilterSelector( + COMMON_SELECTORS.STATUS_FILTER_TEXT.ALL, + ), + success: COMMON_SELECTORS.statusFilterSelector( + COMMON_SELECTORS.STATUS_FILTER_TEXT.SUCCESS, + ), + failed: COMMON_SELECTORS.statusFilterSelector( + COMMON_SELECTORS.STATUS_FILTER_TEXT.FAILED, + ), + inconclusive: COMMON_SELECTORS.statusFilterSelector( + COMMON_SELECTORS.STATUS_FILTER_TEXT.INCONCLUSIVE, + ), + }, + detailsButton: '[data-test-id="details-button"]', + }, + + testsTable: { + statusFilters: { + all: COMMON_SELECTORS.statusFilterSelector( + COMMON_SELECTORS.STATUS_FILTER_TEXT.ALL, + ), + success: COMMON_SELECTORS.statusFilterSelector( + COMMON_SELECTORS.STATUS_FILTER_TEXT.SUCCESS, + ), + failed: COMMON_SELECTORS.statusFilterSelector( + COMMON_SELECTORS.STATUS_FILTER_TEXT.FAILED, + ), + inconclusive: COMMON_SELECTORS.statusFilterSelector( + COMMON_SELECTORS.STATUS_FILTER_TEXT.INCONCLUSIVE, + ), + }, + testItem: 'tr', + expandedRows: 'tr:has(td[colspan])', + detailsButton: '[data-test-id="details-button"]', + }, + + configTable: { + link: (config: string) => `a:has-text("${config}")`, + }, + + commitGraph: { + container: '[data-test-id="commit-navigation-graph"]', + svg: '[data-test-id="commit-navigation-graph"] svg', + marks: '[data-test-id="commit-navigation-graph"] [class*="MuiMarkElement"]', + }, +} as const; diff --git a/dashboard/e2e/e2e-selectors.ts b/dashboard/e2e/selectors/tree-listing.ts similarity index 70% rename from dashboard/e2e/e2e-selectors.ts rename to dashboard/e2e/selectors/tree-listing.ts index 51e170ce3..ca612fd8a 100644 --- a/dashboard/e2e/e2e-selectors.ts +++ b/dashboard/e2e/selectors/tree-listing.ts @@ -5,7 +5,6 @@ export const TREE_LISTING_SELECTORS = { intervalInput: 'input[type="number"][min="1"]', - // This requires nth() selector which can't be stored as string itemsPerPageDropdown: '[role="listbox"]', itemsPerPageOption: (value: string) => `[role="option"]:has-text("${value}")`, @@ -19,11 +18,3 @@ export const TREE_LISTING_SELECTORS = { breadcrumbTreesLink: '[data-test-id="breadcrumb-link"]:has-text("Trees")', } as const; - -export const COMMON_SELECTORS = { - tableRow: 'tr', - tableHeader: 'th', - - originDropdown: '[data-test-id="origin-dropdown"]', - originOption: (origin: string) => `[data-test-id="origin-option-${origin}"]`, -} as const; diff --git a/dashboard/e2e/tree-details.spec.ts b/dashboard/e2e/tree-details.spec.ts new file mode 100644 index 000000000..6c9520f2b --- /dev/null +++ b/dashboard/e2e/tree-details.spec.ts @@ -0,0 +1,324 @@ +import { test, expect } from '@playwright/test'; + +import { TREE_DETAILS_SELECTORS, TREE_LISTING_SELECTORS } from './selectors'; +import { + navigateAndWait, + navigateBackViaBreadcrumb, + clickAndVerifyNavigation, + withFilterDrawer, +} from './utils'; + +const EXPAND_ROW_TIMEOUT = 3000; +const GRAPH_RENDER_TIMEOUT = 2000; +const CLEAR_FILTER_TIMEOUT = 1000; + +test.describe('Tree Details Page Tests', () => { + const TREE_URL = + 'tree/android/android-mainline/7c5912a7c78d669e825093b8cbb57e6afaa88d11'; + + test.beforeEach(async ({ page }) => { + const timeout = 60000; + page.setDefaultTimeout(timeout); + }); + + test('adds filters via filter drawer and applies them', async ({ page }) => { + await navigateAndWait(page, TREE_URL); + + // Open filter drawer + const filterButton = page.locator(TREE_DETAILS_SELECTORS.filters.button); + await expect(filterButton).toBeVisible({ timeout: 30000 }); + await filterButton.click(); + + // Wait for drawer to be visible + const drawerContent = page.locator( + TREE_DETAILS_SELECTORS.filters.drawerContent, + ); + await expect(drawerContent).toBeVisible(); + + // Select FAIL filter in Build Status (first FAIL label in the page) + const failCheckbox = page.locator('label:has-text("FAIL")').first(); + await expect(failCheckbox).toBeVisible(); + await failCheckbox.click(); + + // Apply filter by clicking Filter button + const applyFilterButton = page.locator( + TREE_DETAILS_SELECTORS.filters.filterButton, + ); + await expect(applyFilterButton).toBeVisible(); + await applyFilterButton.click(); + + // Wait for drawer to close + await expect(drawerContent).not.toBeVisible(); + + // Verify URL contains filter parameter + await expect(page).toHaveURL(/df.*FAIL/); + + // Verify filter was applied by checking if build table shows filtered results + await page.waitForLoadState('domcontentloaded'); + + // Open filter drawer again to clear filters + await filterButton.click(); + await expect(drawerContent).toBeVisible(); + + // Clear all filters if available + const clearAllButton = page.locator( + TREE_DETAILS_SELECTORS.filters.clearAllFilters, + ); + const hasClearAllButton = (await clearAllButton.count()) > 0; + + if (hasClearAllButton) { + await clearAllButton.click(); + await page.waitForTimeout(CLEAR_FILTER_TIMEOUT); + } + + // Close drawer + const cancelButton = page.locator( + TREE_DETAILS_SELECTORS.filters.cancelButton, + ); + await cancelButton.click(); + await expect(drawerContent).not.toBeVisible(); + }); + + test('clicks on details button on build table and goes back via breadcrumb', async ({ + page, + }) => { + await navigateAndWait(page, TREE_URL); + + const detailsButton = page + .locator(TREE_DETAILS_SELECTORS.buildTable.detailsButton) + .first(); + await expect(detailsButton).toBeVisible({ timeout: 30000 }); + + await clickAndVerifyNavigation(detailsButton, /\/build\//); + + await navigateBackViaBreadcrumb( + page, + TREE_DETAILS_SELECTORS.breadcrumbTreesLink, + ); + }); + + test('clicks on an issue details button on issue card and goes back via breadcrumb', async ({ + page, + }) => { + await navigateAndWait(page, TREE_URL); + + const issuesCardButton = page.locator( + TREE_DETAILS_SELECTORS.issuesCard.button, + ); + const hasIssues = (await issuesCardButton.count()) > 0; + + if (hasIssues) { + await expect(issuesCardButton).toBeVisible(); + + await clickAndVerifyNavigation(issuesCardButton, /\/issues\//); + + await navigateBackViaBreadcrumb( + page, + TREE_DETAILS_SELECTORS.breadcrumbTreesLink, + ); + } + }); + + test('selects other tabs', async ({ page }) => { + await navigateAndWait(page, TREE_URL); + + const bootsTab = page.locator(TREE_DETAILS_SELECTORS.tabs.boots); + await expect(bootsTab).toBeVisible(); + + await bootsTab.click(); + await expect(bootsTab).toHaveAttribute('data-state', 'active'); + + const testsTab = page.locator(TREE_DETAILS_SELECTORS.tabs.tests); + await expect(testsTab).toBeVisible(); + + await testsTab.click(); + await expect(testsTab).toHaveAttribute('data-state', 'active'); + + const buildsTab = page.locator(TREE_DETAILS_SELECTORS.tabs.builds); + await buildsTab.click(); + await expect(buildsTab).toHaveAttribute('data-state', 'active'); + }); + + test('clicks on a test item and clicks on detail button for test item', async ({ + page, + }) => { + await navigateAndWait(page, TREE_URL); + + const testsTab = page.locator(TREE_DETAILS_SELECTORS.tabs.tests); + await expect(testsTab).toBeVisible(); + await testsTab.click(); + + await page.waitForLoadState('domcontentloaded'); + + const testItems = page.locator(TREE_DETAILS_SELECTORS.testsTable.testItem); + const hasTestItems = (await testItems.count()) > 0; + + if (hasTestItems) { + const firstTestItem = testItems.first(); + await firstTestItem.click(); + + await page.waitForTimeout(EXPAND_ROW_TIMEOUT); + + const expandedRows = page.locator( + TREE_DETAILS_SELECTORS.testsTable.expandedRows, + ); + const hasExpandedRows = (await expandedRows.count()) > 0; + + if (hasExpandedRows) { + const detailsButton = page + .locator(TREE_DETAILS_SELECTORS.testsTable.detailsButton) + .first(); + await expect(detailsButton).toBeVisible({ timeout: 10000 }); + + await clickAndVerifyNavigation(detailsButton, /\/test\//); + + await navigateBackViaBreadcrumb( + page, + TREE_DETAILS_SELECTORS.breadcrumbTreesLink, + ); + } + } + }); + + test('full workflow: navigate to tree details and back via breadcrumb', async ({ + page, + }) => { + await navigateAndWait(page, '/tree'); + + await expect(page).toHaveURL(/\/tree$/); + + const firstTreeLink = page + .locator(TREE_LISTING_SELECTORS.firstTreeCell) + .first(); + await expect(firstTreeLink).toBeVisible(); + + await clickAndVerifyNavigation( + firstTreeLink, + /\/tree\/[^/]+\/[^/]+\/[^/]+$/, + ); + + await navigateBackViaBreadcrumb( + page, + TREE_DETAILS_SELECTORS.breadcrumbTreesLink, + ); + }); + + test('build table status filters work correctly', async ({ page }) => { + await navigateAndWait(page, TREE_URL); + + const allFilter = page.locator( + TREE_DETAILS_SELECTORS.buildTable.statusFilters.all, + ); + await expect(allFilter).toBeVisible({ timeout: 30000 }); + + const successFilter = page.locator( + TREE_DETAILS_SELECTORS.buildTable.statusFilters.success, + ); + await expect(successFilter).toBeVisible(); + + await successFilter.click(); + + await allFilter.click(); + }); + + test('search input is visible and functional', async ({ page }) => { + await navigateAndWait(page, TREE_URL); + + const searchInput = page.locator( + TREE_DETAILS_SELECTORS.buildTable.searchInput, + ); + await expect(searchInput).toBeVisible({ timeout: 30000 }); + + await searchInput.fill('defconfig'); + + await expect(searchInput).toHaveValue('defconfig'); + }); + + test('navigates back one commit via commit graph', async ({ page }) => { + await navigateAndWait(page, TREE_URL); + + const currentUrl = page.url(); + + // Wait for a visible commit graph (there may be multiple, get the last visible one) + const commitGraphContainer = page + .locator(TREE_DETAILS_SELECTORS.commitGraph.container) + .last(); + await expect(commitGraphContainer).toBeVisible({ timeout: 30000 }); + + // Wait a bit for the graph to fully render + await page.waitForTimeout(GRAPH_RENDER_TIMEOUT); + + // Look for clickable marks in the visible graph + const marks = commitGraphContainer.locator('[class*="MuiMarkElement"]'); + const markCount = await marks.count(); + + // Ensure we actually have marks to click + expect(markCount).toBeGreaterThan(0); + + // Click on the first mark (data point in the commit history) + await marks.first().click({ force: true }); + + // Wait for navigation to complete + await page.waitForTimeout(GRAPH_RENDER_TIMEOUT); + + const newUrl = page.url(); + + // Verify URL changed (navigated to a different commit) + expect(newUrl).not.toBe(currentUrl); + expect(newUrl).toMatch(/\/tree\//); + }); + + test('adds filter via summary card and verifies URL change', async ({ + page, + }) => { + await navigateAndWait(page, TREE_URL); + + // Wait for summary cards to load + await page.waitForLoadState('domcontentloaded'); + + // Find and click on an arch filter link (e.g., arm64) + const arm64Link = page.locator('a:has-text("arm64")').first(); + const hasArm64Link = (await arm64Link.count()) > 0; + + if (hasArm64Link) { + await expect(arm64Link).toBeVisible({ timeout: 30000 }); + + // Get current URL before clicking + const urlBefore = page.url(); + + // Click the link + await arm64Link.click(); + + // Wait for navigation + await page.waitForLoadState('domcontentloaded'); + + // Verify URL changed and contains filter parameter + const urlAfter = page.url(); + expect(urlAfter).not.toBe(urlBefore); + expect(urlAfter).toMatch(/df.*arm64/); + } + }); + + test('clears all filters', async ({ page }) => { + await navigateAndWait(page, TREE_URL); + + await withFilterDrawer( + page, + TREE_DETAILS_SELECTORS.filters.button, + TREE_DETAILS_SELECTORS.filters.cancelButton, + TREE_DETAILS_SELECTORS.filters.drawerContent, + async () => { + // Check if "Clear all" button exists and click it + const clearAllButton = page.locator( + TREE_DETAILS_SELECTORS.filters.clearAllFilters, + ); + const hasClearAllButton = (await clearAllButton.count()) > 0; + + if (hasClearAllButton) { + await clearAllButton.click(); + await page.waitForTimeout(CLEAR_FILTER_TIMEOUT); + } + }, + ); + }); +}); diff --git a/dashboard/e2e/tree-listing.spec.ts b/dashboard/e2e/tree-listing.spec.ts index ae37e5990..77348928e 100644 --- a/dashboard/e2e/tree-listing.spec.ts +++ b/dashboard/e2e/tree-listing.spec.ts @@ -1,12 +1,16 @@ import { test, expect } from '@playwright/test'; -import { TREE_LISTING_SELECTORS, COMMON_SELECTORS } from './e2e-selectors'; +import { + TREE_LISTING_SELECTORS, + COMMON_SELECTORS, + TREE_DETAILS_SELECTORS, +} from './selectors'; +import { navigateBackViaBreadcrumb, clickAndVerifyNavigation } from './utils'; const PAGE_LOAD_TIMEOUT = 5000; const DEFAULT_ACTION_TIMEOUT = 1000; const SEARCH_UPDATE_TIMEOUT = 2000; const NAVIGATION_TIMEOUT = 5000; -const GO_BACK_TIMEOUT = 3000; test.describe('Tree Listing Page Tests', () => { test.beforeEach(async ({ page }) => { @@ -80,21 +84,17 @@ test.describe('Tree Listing Page Tests', () => { const firstTreeLink = page.locator('td a').first(); await expect(firstTreeLink).toBeVisible(); - await firstTreeLink.click(); - - await page.waitForTimeout(NAVIGATION_TIMEOUT); - - const url = page.url(); - expect(url).toMatch(/\/tree\/[^/]+\/[^/]+\/[^/]+$/); - - const breadcrumbLink = page.locator( - TREE_LISTING_SELECTORS.breadcrumbTreesLink, + await clickAndVerifyNavigation( + firstTreeLink, + /\/tree\/[^/]+\/[^/]+\/[^/]+$/, + { waitAfterClick: NAVIGATION_TIMEOUT }, ); - await expect(breadcrumbLink).toBeVisible({ timeout: 15000 }); - await breadcrumbLink.click(); - await page.waitForTimeout(GO_BACK_TIMEOUT); - await expect(page).toHaveURL(/\/tree$/); + await navigateBackViaBreadcrumb( + page, + TREE_DETAILS_SELECTORS.breadcrumbTreesLink, + { expectedUrl: /\/tree$/ }, + ); }); test('pagination navigation', async ({ page }) => { diff --git a/dashboard/e2e/utils/filters.ts b/dashboard/e2e/utils/filters.ts new file mode 100644 index 000000000..7e2763887 --- /dev/null +++ b/dashboard/e2e/utils/filters.ts @@ -0,0 +1,60 @@ +import type { Page } from '@playwright/test'; +import { expect } from '@playwright/test'; + +const DRAWER_TIMEOUT = 10000; + +/** + * Open the filter drawer and wait for it to be visible + */ +export async function openFilterDrawer( + page: Page, + filterButtonSelector: string, + drawerContentSelector: string, + options?: { + timeout?: number; + }, +): Promise { + const timeout = options?.timeout ?? DRAWER_TIMEOUT; + + const filterButton = page.locator(filterButtonSelector); + await filterButton.waitFor({ state: 'visible', timeout }); + await filterButton.click(); + + const drawerContent = page.locator(drawerContentSelector); + await expect(drawerContent).toBeVisible({ timeout }); +} + +/** + * Close the filter drawer by clicking the cancel button + */ +export async function closeFilterDrawer( + page: Page, + cancelButtonSelector: string, + drawerContentSelector: string, +): Promise { + const cancelButton = page.locator(cancelButtonSelector); + await expect(cancelButton).toBeVisible(); + await cancelButton.click(); + + const drawerContent = page.locator(drawerContentSelector); + await expect(drawerContent).not.toBeVisible(); +} + +/** + * Open filter drawer, optionally perform actions, then close it + */ +export async function withFilterDrawer( + page: Page, + filterButtonSelector: string, + cancelButtonSelector: string, + drawerContentSelector: string, + action?: (page: Page) => Promise, +): Promise { + await openFilterDrawer(page, filterButtonSelector, drawerContentSelector); + + if (action) { + await action(page); + } + + await closeFilterDrawer(page, cancelButtonSelector, drawerContentSelector); +} diff --git a/dashboard/e2e/utils/index.ts b/dashboard/e2e/utils/index.ts new file mode 100644 index 000000000..196277885 --- /dev/null +++ b/dashboard/e2e/utils/index.ts @@ -0,0 +1,2 @@ +export * from './navigation'; +export * from './filters'; diff --git a/dashboard/e2e/utils/navigation.ts b/dashboard/e2e/utils/navigation.ts new file mode 100644 index 000000000..6c04da3eb --- /dev/null +++ b/dashboard/e2e/utils/navigation.ts @@ -0,0 +1,74 @@ +import type { Page, Locator } from '@playwright/test'; +import { expect } from '@playwright/test'; + +const NAVIGATE_TIMEOUT = 30000; +const BREADCRUMB_TIMEOUT = 20000; + +/** + * Navigate to a page and wait for it to be fully loaded + */ +export async function navigateAndWait( + page: Page, + url: string, + options?: { + timeout?: number; + waitForLoadState?: boolean; + }, +): Promise { + const timeout = options?.timeout ?? NAVIGATE_TIMEOUT; + const waitForLoadState = options?.waitForLoadState ?? true; + + await page.goto(url, { timeout }); + + if (waitForLoadState) { + await page.waitForLoadState('domcontentloaded'); + } +} + +/** + * Click a breadcrumb link and navigate back to tree listing + */ +export async function navigateBackViaBreadcrumb( + page: Page, + breadcrumbSelector: string, + options?: { + expectedUrl?: RegExp; + timeout?: number; + }, +): Promise { + const timeout = options?.timeout ?? BREADCRUMB_TIMEOUT; + const expectedUrl = options?.expectedUrl ?? /\/tree/; + + const breadcrumbLink = page.locator(breadcrumbSelector); + await expect(breadcrumbLink).toBeVisible(); + await breadcrumbLink.click(); + + await page.waitForURL(expectedUrl, { timeout }); + await expect(page).toHaveURL(expectedUrl); +} + +/** + * Click an element, wait for navigation, and verify URL pattern + */ +export async function clickAndVerifyNavigation( + element: Locator, + urlPattern: RegExp, + options?: { + timeout?: number; + waitAfterClick?: number; + }, +): Promise { + const timeout = options?.timeout ?? NAVIGATE_TIMEOUT; + const waitAfterClick = options?.waitAfterClick ?? 0; + + await element.click(); + + if (waitAfterClick > 0) { + await element.page().waitForTimeout(waitAfterClick); + } + + await element.page().waitForURL(urlPattern, { timeout }); + + const url = element.page().url(); + expect(url).toMatch(urlPattern); +} diff --git a/dashboard/eslint.config.mjs b/dashboard/eslint.config.mjs index a3014444c..0c450cf0e 100644 --- a/dashboard/eslint.config.mjs +++ b/dashboard/eslint.config.mjs @@ -138,6 +138,7 @@ export default [{ "src/stories/**", "**/*.stories*", "playwright.config.ts", + "e2e/**/*.{ts,tsx}", ], }], diff --git a/dashboard/package.json b/dashboard/package.json index 51cf22f00..80c9fb030 100644 --- a/dashboard/package.json +++ b/dashboard/package.json @@ -5,6 +5,7 @@ "type": "module", "scripts": { "dev": "rm -rf ./node_modules/.vite && vite", + "dev-remote-api": "rm -rf ./node_modules/.vite && VITE_API_BASE_URL=/staging-api vite", "build": "tsc -b && vite build", "test": "vitest", "coverage": "vitest run --coverage", diff --git a/dashboard/src/components/BootsTable/BootsTable.tsx b/dashboard/src/components/BootsTable/BootsTable.tsx index 1fd911d48..dc884a729 100644 --- a/dashboard/src/components/BootsTable/BootsTable.tsx +++ b/dashboard/src/components/BootsTable/BootsTable.tsx @@ -119,6 +119,9 @@ const defaultColumns: ColumnDef[] = [ id: DETAILS_COLUMN_ID, header: (): JSX.Element => , cell: (): JSX.Element => , + meta: { + dataTestId: 'details-button', + }, }, ]; diff --git a/dashboard/src/components/Breadcrumb/Breadcrumb.tsx b/dashboard/src/components/Breadcrumb/Breadcrumb.tsx index 6b66a6211..f91058b8d 100644 --- a/dashboard/src/components/Breadcrumb/Breadcrumb.tsx +++ b/dashboard/src/components/Breadcrumb/Breadcrumb.tsx @@ -28,12 +28,12 @@ type BreadcrumbLinkProps = Pick< const BreadcrumbLink = ({ children, ...props -}: BreadcrumbLinkProps): JSX.Element => { +}: BreadcrumbLinkProps & { 'data-test-id'?: string }): JSX.Element => { return ( {children} diff --git a/dashboard/src/components/Breadcrumb/TreeBreadcrumb.tsx b/dashboard/src/components/Breadcrumb/TreeBreadcrumb.tsx index cf0f3d173..6144a4e51 100644 --- a/dashboard/src/components/Breadcrumb/TreeBreadcrumb.tsx +++ b/dashboard/src/components/Breadcrumb/TreeBreadcrumb.tsx @@ -60,7 +60,11 @@ const TreeBreadcrumb = ({ - + diff --git a/dashboard/src/components/BuildsTable/DefaultBuildsColumns.tsx b/dashboard/src/components/BuildsTable/DefaultBuildsColumns.tsx index 0697e866d..0d380c23f 100644 --- a/dashboard/src/components/BuildsTable/DefaultBuildsColumns.tsx +++ b/dashboard/src/components/BuildsTable/DefaultBuildsColumns.tsx @@ -111,5 +111,8 @@ export const defaultBuildColumns: ColumnDef[] = [ id: DETAILS_COLUMN_ID, header: (): JSX.Element => , cell: (): JSX.Element => , + meta: { + dataTestId: 'details-button', + }, }, ]; diff --git a/dashboard/src/components/Cards/BaseCard.tsx b/dashboard/src/components/Cards/BaseCard.tsx index be28cb8df..5992c6852 100644 --- a/dashboard/src/components/Cards/BaseCard.tsx +++ b/dashboard/src/components/Cards/BaseCard.tsx @@ -5,6 +5,7 @@ import { cn } from '@/lib/utils'; type CommonProps = { title: ReactNode; className?: string; + 'data-test-id'?: string; }; type WithChildren = { @@ -24,6 +25,7 @@ export const BaseCard = ({ content, className, children, + 'data-test-id': dataTestId, }: IBaseCard): JSX.Element => { return (
{title} diff --git a/dashboard/src/components/CommitNavigationGraph/CommitNavigationGraph.tsx b/dashboard/src/components/CommitNavigationGraph/CommitNavigationGraph.tsx index c28b42ddf..2dfce1657 100644 --- a/dashboard/src/components/CommitNavigationGraph/CommitNavigationGraph.tsx +++ b/dashboard/src/components/CommitNavigationGraph/CommitNavigationGraph.tsx @@ -242,6 +242,7 @@ const CommitNavigationGraph = ({ title={formatMessage({ id: messagesId.graphName })} content={ - @@ -184,7 +188,11 @@ const Drawer = ({ asChild className="bg-blue w-[200px] rounded-full text-white" > - diff --git a/dashboard/src/components/LineChart/LineChart.tsx b/dashboard/src/components/LineChart/LineChart.tsx index 5e9dcf63d..6c4b1f829 100644 --- a/dashboard/src/components/LineChart/LineChart.tsx +++ b/dashboard/src/components/LineChart/LineChart.tsx @@ -34,6 +34,7 @@ export type TLineChartProps = { onMarkClick?: MUILineChartProps['onMarkClick']; slots?: MUILineChartProps['slots']; slotProps?: MUILineChartProps['slotProps']; + dataTestId?: string; }; export const LineChart = ({ @@ -44,9 +45,10 @@ export const LineChart = ({ slotProps, sx, onMarkClick, + dataTestId, }: TLineChartProps): JSX.Element => { return ( -
+
{labels && (
{labels}
)} diff --git a/dashboard/src/components/StatusChart/StatusCharts.tsx b/dashboard/src/components/StatusChart/StatusCharts.tsx index 75c3befca..06a973ba2 100644 --- a/dashboard/src/components/StatusChart/StatusCharts.tsx +++ b/dashboard/src/components/StatusChart/StatusCharts.tsx @@ -158,6 +158,7 @@ const ChartLegend = ({ chartValues, onClick }: IChartLegend): JSX.Element => { onClick={(): void => onClick?.(status)} key={chartValue?.color} className="flex flex-row text-left" + data-test-id={chartValue?.label} > {chartValue && (
diff --git a/dashboard/src/components/Table/TableComponents.tsx b/dashboard/src/components/Table/TableComponents.tsx index de0d57fa2..d6ea5b014 100644 --- a/dashboard/src/components/Table/TableComponents.tsx +++ b/dashboard/src/components/Table/TableComponents.tsx @@ -2,6 +2,7 @@ import type { LinkProps } from '@tanstack/react-router'; import type { Cell, Row } from '@tanstack/react-table'; import { flexRender } from '@tanstack/react-table'; import { memo, useCallback, useMemo, type JSX } from 'react'; +import { z } from 'zod'; import { TableCellWithLink, TableRow } from '@/components/ui/table'; @@ -9,6 +10,10 @@ import { cn } from '@/lib/utils'; import { DETAILS_COLUMN_ID } from './DetailsColumn'; +const ColumnMetaSchema = z.object({ + dataTestId: z.string().optional(), +}); + type BaseComponentType = { id: string; }; @@ -38,11 +43,17 @@ const TableCellComponent = ({ ? detailsLinkProps : { search: s => s, state: s => s }; + const metaResult = ColumnMetaSchema.safeParse(cell.column.columnDef.meta); + const dataTestId = metaResult.success + ? metaResult.data.dataTestId + : undefined; + return ( {flexRender(cell.column.columnDef.cell, cell.getContext())} diff --git a/dashboard/src/components/Tabs/StatusCard.tsx b/dashboard/src/components/Tabs/StatusCard.tsx index 73e06a260..cc43ecddb 100644 --- a/dashboard/src/components/Tabs/StatusCard.tsx +++ b/dashboard/src/components/Tabs/StatusCard.tsx @@ -85,6 +85,7 @@ const StatusCard = ({ return ( [] = [ id: DETAILS_COLUMN_ID, header: (): JSX.Element => , cell: (): JSX.Element => , + meta: { + dataTestId: 'details-button', + }, }, ]; diff --git a/dashboard/src/components/ui/table.tsx b/dashboard/src/components/ui/table.tsx index 5204fb664..af71de6bf 100644 --- a/dashboard/src/components/ui/table.tsx +++ b/dashboard/src/components/ui/table.tsx @@ -103,22 +103,32 @@ type TableCellWithLinkAttributes = React.TdHTMLAttributes & { linkProps: RouterLinkProps; linkClassName?: string; + dataTestId?: string; }; const TableCellWithLink = React.forwardRef< HTMLTableCellElement, TableCellWithLinkAttributes ->(({ className, children, linkProps, linkClassName, ...props }, ref) => ( - - - {children} - - -)); +>( + ( + { className, children, linkProps, linkClassName, dataTestId, ...props }, + ref, + ) => ( + + + {children} + + + ), +); TableCellWithLink.displayName = 'TableCellWithLink'; const TableCaption = React.forwardRef< diff --git a/dashboard/src/pages/TreeDetails/TreeDetails.tsx b/dashboard/src/pages/TreeDetails/TreeDetails.tsx index ce9e1f404..92d4482aa 100644 --- a/dashboard/src/pages/TreeDetails/TreeDetails.tsx +++ b/dashboard/src/pages/TreeDetails/TreeDetails.tsx @@ -397,6 +397,7 @@ const TreeDetails = ({ }; }} state={s => s} + data-test-id="breadcrumb-trees-link" > diff --git a/dashboard/src/pages/hardwareDetails/Tabs/Boots/HardwareDetailsBootsTable.tsx b/dashboard/src/pages/hardwareDetails/Tabs/Boots/HardwareDetailsBootsTable.tsx index 5c5674245..2181cdc1f 100644 --- a/dashboard/src/pages/hardwareDetails/Tabs/Boots/HardwareDetailsBootsTable.tsx +++ b/dashboard/src/pages/hardwareDetails/Tabs/Boots/HardwareDetailsBootsTable.tsx @@ -87,6 +87,9 @@ export const columns: ColumnDef[] = [ id: DETAILS_COLUMN_ID, header: (): JSX.Element => , cell: (): JSX.Element => , + meta: { + dataTestId: 'details-button', + }, }, ]; diff --git a/dashboard/src/pages/hardwareDetails/Tabs/Tests/HardwareDetailsTestsTable.tsx b/dashboard/src/pages/hardwareDetails/Tabs/Tests/HardwareDetailsTestsTable.tsx index 87b28c61e..546502523 100644 --- a/dashboard/src/pages/hardwareDetails/Tabs/Tests/HardwareDetailsTestsTable.tsx +++ b/dashboard/src/pages/hardwareDetails/Tabs/Tests/HardwareDetailsTestsTable.tsx @@ -86,6 +86,9 @@ const innerColumns: ColumnDef[] = [ id: DETAILS_COLUMN_ID, header: (): JSX.Element => , cell: (): JSX.Element => , + meta: { + dataTestId: 'details-button', + }, }, ]; diff --git a/dashboard/vite.config.ts b/dashboard/vite.config.ts index a0ca62a28..5ee537970 100644 --- a/dashboard/vite.config.ts +++ b/dashboard/vite.config.ts @@ -14,4 +14,14 @@ export default defineConfig({ '@': path.resolve(__dirname, './src'), }, }, + server: { + proxy: { + '/staging-api': { + target: 'https://staging.dashboard.kernelci.org:9000', + changeOrigin: true, + secure: false, + rewrite: urlPath => urlPath.replace(/^\/staging-api/, ''), + }, + }, + }, });