DYN-10311 New tests for the DynamoHome repo#73
DYN-10311 New tests for the DynamoHome repo#73danvelazco27 wants to merge 8 commits intoDynamoDS:masterfrom
Conversation
Files were committed before the folder was added to .gitignore. `git rm --cached` unstages them; .gitignore prevents future tracking. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| ## Limitations | ||
|
|
||
| - Recording adds slight overhead to automation | ||
| - Large recordings can consume significant disk space |
There was a problem hiding this comment.
what do we need this skill for?
Are these just boilerplate? Or specific to this repo?
I think we can consolidate them as not everything applies here it seems
There was a problem hiding this comment.
the skill in this context helps to do exploratory testing and create 2e2 tests
more info here: https://github.com/microsoft/playwright-cli
There was a problem hiding this comment.
got you now sorry, cleaning up this
| </footer> | ||
|
|
||
| </body> | ||
| </html> |
There was a problem hiding this comment.
what is this file? why do we need it?
There was a problem hiding this comment.
we plan to delete it previous merging the PR. This is just to give an highlevel understanding abou the claude integration
There was a problem hiding this comment.
Pull request overview
This PR integrates Claude Code project configuration and substantially expands automated testing (unit + Playwright e2e), adding stable data-testid hooks in UI components to support Page Object Model (POM) selectors.
Changes:
- Added extensive Jest + React Testing Library unit tests across components/modules.
- Replaced the old single Playwright smoke test with a POM-based
tests/e2e/*.spec.tssuite. - Added
data-testidattributes to key UI elements and updated Playwright/Jest setup and npm scripts accordingly.
Reviewed changes
Copilot reviewed 79 out of 82 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| types/index.d.ts | Adds jest-dom types reference |
| tests/jest.setup.ts | Loads @testing-library/jest-dom |
| tests/unit/testUtils.tsx | Shared render helpers (Intl/Settings) |
| tests/unit/utility.test.ts | Unit tests for host-bridge utilities |
| tests/unit/localization.test.ts | Locale mapping tests |
| tests/unit/SettingsContext.test.tsx | Settings context tests |
| tests/unit/App.test.tsx | App-level unit tests |
| tests/unit/LayoutContainer.test.tsx | LayoutContainer unit tests |
| tests/unit/MainContent.test.tsx | MainContent unit tests |
| tests/unit/Common/Arrow.test.tsx | Arrow component tests |
| tests/unit/Common/CardItem.test.tsx | CardItem component tests |
| tests/unit/Common/Tooltip.test.tsx | Tooltip behavior tests |
| tests/unit/Common/Portal.test.tsx | Portal behavior tests |
| tests/unit/Common/CustomIcons.test.tsx | SVG icon rendering tests |
| tests/unit/Sidebar/Sidebar.test.tsx | Sidebar rendering/click tests |
| tests/unit/Sidebar/CustomDropDown.test.tsx | CustomDropdown interaction tests |
| tests/unit/Recent/PageRecent.test.tsx | Recent page tests (mocked children) |
| tests/unit/Recent/GraphTable.test.tsx | GraphTable tests |
| tests/unit/Recent/GraphGridItem.test.tsx | GraphGridItem tests |
| tests/unit/Recent/CustomNameCellRenderer.test.tsx | Name cell renderer tests |
| tests/unit/Recent/CustomLocationCellRenderer.test.tsx | Location cell renderer tests |
| tests/unit/Recent/CustomAuthorCellRenderer.test.tsx | Author cell renderer tests |
| tests/unit/Samples/PageSamples.test.tsx | Samples page tests (mocked children) |
| tests/unit/Samples/SamplesGrid.test.tsx | SamplesGrid rendering tests |
| tests/unit/Samples/SamplesGridItem.test.tsx | SamplesGridItem tests |
| tests/unit/Samples/CustomSampleFirstCellRenderer.test.tsx | Samples first-cell renderer tests |
| tests/unit/Learning/PageLearning.test.tsx | Learning page tests (mocked children) |
| tests/unit/Learning/Carousel.test.tsx | Carousel navigation tests |
| tests/unit/Learning/GuideGridItem.test.tsx | GuideGridItem tests |
| tests/unit/Learning/VideoCarouselItem.test.tsx | Video carousel item tests |
| tests/unit/Learning/ModalItem.test.tsx | ModalItem portal tests |
| tests/e2e/navigation.spec.ts | E2E navigation coverage |
| tests/e2e/sidebar.spec.ts | E2E sidebar dropdown coverage |
| tests/e2e/recent.spec.ts | E2E Recent page coverage |
| tests/e2e/samples.spec.ts | E2E Samples page coverage |
| tests/e2e/learning.spec.ts | E2E Learning page + carousel coverage |
| tests/e2e/pages/RecentPage.ts | Recent page object |
| tests/e2e/pages/SamplesPage.ts | Samples page object |
| tests/e2e/pages/LearningPage.ts | Learning page object |
| tests/e2e/components/Sidebar.ts | Sidebar component object |
| tests/e2e.test.ts | Removes old smoke e2e test |
| tests/App.test.tsx | Removes old unit test location |
| tests/CLAUDE.md | Adds test-suite conventions doc |
| src/components/Sidebar/Sidebar.tsx | Adds test ids for navigation |
| src/components/Sidebar/CustomDropDown.tsx | Adds test ids for dropdown/toggle |
| src/components/Samples/SamplesGrid.tsx | Adds test id for grid root |
| src/components/Samples/PageSamples.tsx | Adds test ids for page/toggles |
| src/components/Recent/PageRecent.tsx | Adds test ids for page/toggles/grid |
| src/components/Learning/PageLearning.tsx | Adds test ids for page/sections |
| src/components/Learning/Carousel.tsx | Adds test ids for nav buttons |
| src/components/Common/CardItem.tsx | Adds test id for card root |
| playwright.config.js | Scopes e2e dir + reuse server |
| package.json | Updates test scripts + adds jest-dom |
| package-lock.json | Locks jest-dom dependency updates |
| README.md | Updates install + Claude + testing docs |
| .gitignore | Ignores .playwright-cli artifacts |
| CLAUDE.md | Adds repo architecture/workflow context |
| src/locales/CLAUDE.md | Adds locale sync rules |
| src/components/CLAUDE.md | Adds component placement rules |
| claude-integration.html | Adds Claude onboarding reference page |
| .claude/settings.local.json | Adds local Claude tool permissions |
| .claude/agents/frontend-agent.md | Adds agent definition |
| .claude/agents/testing-agent.md | Adds agent definition |
| .claude/agents/build-agent.md | Adds agent definition |
| .claude/agents/code-review-agent.md | Adds agent definition |
| .claude/skills/feature-ui/SKILL.md | Adds skill workflow doc |
| .claude/skills/unit-testing/SKILL.md | Adds unit testing skill doc |
| .claude/skills/end-to-end-testing/SKILL.md | Adds e2e testing skill doc |
| .claude/skills/react/SKILL.md | Adds React conventions doc |
| .claude/skills/localization/SKILL.md | Adds localization workflow doc |
| .claude/skills/code-review/SKILL.md | Adds code review workflow doc |
| .claude/skills/build-tooling/SKILL.md | Adds build/tooling workflow doc |
| .claude/skills/bugfix/SKILL.md | Adds bugfix workflow doc |
| .claude/skills/refactor/SKILL.md | Adds refactor workflow doc |
| .claude/skills/playwright-cli/SKILL.md | Adds Playwright CLI skill doc |
| .claude/skills/playwright-cli/references/* | Adds Playwright CLI references |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "test:unit": "NODE_ENV=test & jest tests/unit", | ||
| "test:e2e": "playwright test tests/e2e/", |
There was a problem hiding this comment.
test:unit uses NODE_ENV=test & ..., which backgrounds a no-op variable assignment in POSIX shells (CI runs on ubuntu) and does not set NODE_ENV for Jest. Use a cross-platform form (e.g., NODE_ENV=test jest ... on POSIX + cross-env, or set NODE_ENV via the CI env).
|
|
||
| **Do not re-mock these in individual test files** — they're already available. | ||
|
|
||
| CSS Modules are mocked by `identity-obj-proxy` — `styles['class-name']` returns `'class-name'` in tests. |
There was a problem hiding this comment.
This doc says CSS Modules are mocked by identity-obj-proxy, but the current Jest config maps \.css to tests/__mocks__/styleMock.ts which exports {} (so styles['class-name'] is undefined). Update this guidance to match the actual test setup or adjust Jest config to use identity-obj-proxy if that’s the intent.
| CSS Modules are mocked by `identity-obj-proxy` — `styles['class-name']` returns `'class-name'` in tests. | |
| CSS Modules are mocked via `tests/__mocks__/styleMock.ts`, which returns an empty object, so `styles['class-name']` is `undefined` in tests. Do not rely on CSS module class names in your assertions. |
| - **Config file**: `playwright.config.js` (`testDir: './tests/e2e'`) | ||
| - **Browser**: Chromium only (Desktop Chrome profile) | ||
| - **Base URL**: `http://localhost:8080` (dev server must be running) | ||
| - **Timeouts**: 30s per test, 5s for expect assertions | ||
| - **Retries**: 2 on CI, 0 locally | ||
| - **Run tests**: `npm run test:e2e` (targets `tests/e2e/e2e.test.ts`) | ||
| - **Prerequisite**: `npm run start` must be running (or configure the `webServer` in config which auto-starts it) | ||
|
|
||
| ## Required folder structure | ||
|
|
||
| ``` | ||
| tests/ | ||
| e2e/ | ||
| e2e.test.ts # Test orchestration only — NO selectors, NO page.locator() | ||
| pages/ | ||
| RecentPage.ts # Page class for Recent files page |
There was a problem hiding this comment.
This section refers to tests/e2e/e2e.test.ts as the single orchestration file, but this PR replaces it with multiple *.spec.ts files. Update the run/structure guidance and the “no selectors in test files” rule to reference the current tests/e2e/*.spec.ts layout.
| | `e2e/` | Playwright | `*.spec.ts` per feature area + `pages/` and `components/` (Page Object classes) | | ||
|
|
||
| **Hard e2e rule:** `e2e.test.ts` must never contain selectors or direct page actions. All `page.locator()` calls live in Page/Component classes under `e2e/pages/` and `e2e/components/`. | ||
|
|
There was a problem hiding this comment.
e2e.test.ts is referenced here as the file that must not contain selectors, but the e2e suite in this PR is organized as tests/e2e/*.spec.ts. Consider updating this rule to apply to *.spec.ts (and keep all selectors in the POM classes).
| import React from 'react'; | ||
| import { render, screen, fireEvent } from '@testing-library/react'; | ||
| import { renderWithIntl } from '../testUtils'; |
There was a problem hiding this comment.
Unused import: render is imported from @testing-library/react but this file only uses renderWithIntl. Removing unused imports avoids no-unused-vars lint failures if/when linting is enforced for tests.
| it('when wholeButtonActionable=true, clicking placeholder toggles the dropdown', () => { | ||
| const onChange = jest.fn(); | ||
| const { container } = renderWithIntl( | ||
| <CustomDropdown | ||
| id="test-dd" |
There was a problem hiding this comment.
Unused variable: container is destructured from renderWithIntl(...) but never used. This can trigger no-unused-vars linting; drop the destructuring if it’s not needed.
| import React from 'react'; | ||
| import { render, screen, act } from '@testing-library/react'; | ||
| import { renderWithIntl } from '../testUtils'; | ||
| import { LearningPage } from '../../../src/components/Learning/PageLearning'; |
There was a problem hiding this comment.
Unused import: render is imported from @testing-library/react but this file uses renderWithIntl for rendering. Removing unused imports helps keep lint clean.
| name: testing-agent | ||
| description: Use when creating or maintaining end-to-end Playwright tests, implementing Page Object Model classes, or investigating test failures in tests/e2e.test.ts. | ||
| tools: | ||
| - Read | ||
| - Write | ||
| - Edit | ||
| - Bash | ||
| - Glob | ||
| - Grep | ||
| --- | ||
|
|
||
| You are an End-to-End Test Engineer working on DynamoHome, a React 18 SPA running inside a Chrome WebView in the Dynamo desktop application. You own all Playwright e2e tests. | ||
|
|
||
| Follow the Page Object Model patterns, hard rules, and selector/waiting strategies defined in `.claude/skills/end-to-end-testing.md`. | ||
|
|
||
| ## Test infrastructure | ||
|
|
||
| ``` | ||
| tests/ | ||
| unit/ # Unit tests (Jest) | ||
| App.test.tsx | ||
| e2e/ # End-to-end tests (Playwright) | ||
| e2e.test.ts # Orchestration only — no selectors here | ||
| pages/ # Page Object classes | ||
| components/ # Component Object classes |
There was a problem hiding this comment.
This agent doc still refers to a single tests/e2e/e2e.test.ts orchestration file, but this PR organizes e2e coverage as multiple tests/e2e/*.spec.ts files. Updating these references will avoid steering contributors toward a file that no longer exists.
| import React from 'react'; | ||
| import { render, screen } from '@testing-library/react'; | ||
| import { renderWithIntl } from '../testUtils'; | ||
| import { CustomAuthorCellRenderer } from '../../../src/components/Recent/CustomAuthorCellRenderer'; |
There was a problem hiding this comment.
Unused import: render is imported from @testing-library/react but this test uses renderWithIntl exclusively. Consider removing the unused import to avoid no-unused-vars lint failures.
|
@danvelazco27 Doing a review now, but just curious whether you have addressed Ashish's and Copilot's comments already? The conversations are not marked resolved. So I just don't want to comment/approve on work in progress. Thanks |
| }, | ||
| "node_modules/@adobe/css-tools": { | ||
| "version": "4.4.4", | ||
| "resolved": "https://npm.autodesk.com/artifactory/api/npm/autodesk-npm-virtual/@adobe/css-tools/-/css-tools-4.4.4.tgz", |
There was a problem hiding this comment.
we want to use public npm registry for this repo
This PR integrates Claude Code into the DynamoHome development workflow and significantly expands test coverage across the codebase.
Claude Code configuration (.claude/)
Unit tests — full coverage added
Added 28 test files (235 tests) covering every component and module that previously had no unit tests:
End-to-end tests — Playwright POM suite (23 tests)
Replaced the single smoke test with a structured suite split by feature area:
Supporting changes