DYN-10311 New tests for the DynamoHome repo#73
Merged
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>
zeusongit
reviewed
Mar 27, 2026
zeusongit
reviewed
Mar 27, 2026
Contributor
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.
|
@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 |
avidit
reviewed
Apr 15, 2026
Collaborator
Author
@jasonstratton I'm retaking this work (vacation time), I already addressed all comments; please make your review. I'll be checking your feedback. Thanks :) |
avidit
reviewed
Apr 23, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 72 out of 75 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (3)
tests/unit/utility.test.ts:1
- The mocked host method name
ShowTempalteappears to be a typo (likelyShowTemplate). If the realscriptObjectAPI/mock uses the correct spelling, this test will fail or (worse) will encode the wrong contract. Align the expectation with the actual WebView host method name used bysideBarCommand.
tests/unit/Sidebar/Sidebar.test.tsx:1 - Brand capitalization: 'GitHub' is the correct spelling. Update the expected text (and the underlying localized string if needed) to 'GitHub Repository' to avoid baking the typo into tests.
tests/unit/Sidebar/CustomDropDown.test.tsx:1 - This assertion is brittle because it depends on incidental DOM structure (total
<span>counts) and also acknowledges CSS-module class matching won’t work reliably in tests. Since the component now exposes stabledata-testids for the dropdown root/toggle, consider adding a dedicated identifier for the divider element (e.g.,data-testidorrole=\"separator\") and assert presence/absence directly rather than counting spans.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
@danvelazco27 I would make the lint update a separate PR |
Collaborator
Author
@avidit done |
QilongTang
approved these changes
Apr 24, 2026
Collaborator
|
hmm somehow the build is failing? Copilot is not available to help fix it with a fork |
Collaborator
Author
Reverting some changes by Aabi's suggestion, I broke the test scripts. I already fixed them. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 73 out of 76 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
src/components/Learning/Carousel.tsx:17
maxIndexbecomes negative whentotalItems < itemsPerPage, which can drivecurrentIndexnegative (e.g., clicking Prev at index 0 sets index to a negative maxIndex), producing invalid transforms and wrap behavior. ClampmaxIndexto a minimum of 0 (and optionally disable nav buttons whentotalItems <= itemsPerPage) so navigation cannot move outside the valid range.
export const Carousel = ({ children }: {children: ReactNode}) => {
const [currentIndex, setCurrentIndex] = useState(0);
const itemsPerPage = 4;
const totalItems = React.Children.count(children);
const maxIndex = totalItems - itemsPerPage
const goLeft = () => {
setCurrentIndex(currentIndex === 0 ? maxIndex : currentIndex - 1);
};
const goRight = () => {
setCurrentIndex(currentIndex === maxIndex ? 0 : currentIndex + 1);
}
tests/e2e/navigation.spec.ts:1
- This repo’s documented e2e rule says
*.spec.tsshould avoid direct page actions, but these specs callpage.goto(...)directly. To align with the stated POM convention, consider introducing anApp/Homepage object with agoto()method (and reuse it across specs) so navigation becomes part of the POM layer.
tests/unit/Sidebar/Sidebar.test.tsx:1 - Use the correct capitalization for the brand name: change 'Github' to 'GitHub' (both the test description and the expected text, if the UI is intended to display the brand correctly).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+6
to
7
| <div className={styles['graph-container']} data-testid="card-item"> | ||
| <a className={styles['graph-link']} onClick={onClick}> |
There was a problem hiding this comment.
Using an <a> without an href as an interactive control is not keyboard-accessible by default and can confuse assistive tech semantics. Prefer a <button type=\"button\"> for click actions, or add proper link semantics (href) plus keyboard handlers/ARIA if it truly represents navigation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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