🐛 Fix Auto-QA issues: button patterns and missing test coverage#4172
🐛 Fix Auto-QA issues: button patterns and missing test coverage#4172clubanderson merged 1 commit intomainfrom
Conversation
Replace raw <button> elements in Tour.tsx and Onboarding.tsx with the shared Button component for consistent styling (#4164). Add smoke tests for 12 untested components: Tour, Onboarding, CardHistory, HelmReleases, Deployments, RewardsPanel, GitHubInvite, LinkedInShare, CommandLineTab, ImportTab, PodExecTerminal, SlackNotificationSettings (#4168). Closes #4164 Closes #4168 Signed-off-by: Andrew Anderson <andy@clubanderson.com>
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR addresses Auto-QA findings by standardizing button usage in onboarding UI and adding basic smoke tests to cover previously untested components.
Changes:
- Replaced several raw
<button>elements with the sharedButtoncomponent in onboarding-related UI for consistent styling/behavior. - Added Vitest smoke tests for multiple UI components to close gaps in component-level test coverage.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/components/terminal/tests/PodExecTerminal.test.tsx | Adds a non-rendering smoke test validating the PodExecTerminal export with xterm mocked. |
| web/src/components/settings/sections/tests/SlackNotificationSettings.test.tsx | Adds a render smoke test for Slack notification settings with supporting mocks. |
| web/src/components/rewards/tests/RewardsPanel.test.tsx | Adds an import-based smoke test for RewardsPanel with hooks mocked. |
| web/src/components/rewards/tests/LinkedInShare.test.tsx | Adds render smoke tests for LinkedIn share button/card with hooks mocked. |
| web/src/components/rewards/tests/GitHubInvite.test.tsx | Adds a render smoke test for the GitHub invite button with hooks mocked. |
| web/src/components/onboarding/tests/Tour.test.tsx | Adds render smoke tests for Tour components with tour hook mocked. |
| web/src/components/onboarding/tests/Onboarding.test.tsx | Adds an import-based smoke test for the Onboarding export with routing/auth mocked. |
| web/src/components/onboarding/Tour.tsx | Replaces raw buttons with the shared Button component in tour overlay/trigger UI. |
| web/src/components/onboarding/Onboarding.tsx | Replaces raw buttons with the shared Button component in onboarding navigation/ranking UI. |
| web/src/components/history/tests/CardHistory.test.tsx | Adds a render smoke test for CardHistory with history hook mocked. |
| web/src/components/helm/tests/HelmReleases.test.tsx | Adds an import-based smoke test for HelmReleases with dashboard hooks mocked. |
| web/src/components/deployments/tests/Deployments.test.tsx | Adds an import-based smoke test for Deployments with cached-data hooks mocked. |
| web/src/components/clusters/add-cluster/tests/ImportTab.test.tsx | Adds a render smoke test for ImportTab. |
| web/src/components/clusters/add-cluster/tests/CommandLineTab.test.tsx | Adds a render smoke test for CommandLineTab. |
| )} | ||
| icon={<LogoWithStar className="w-5 h-5" />} | ||
| className={cn(!hasCompletedTour && 'animate-pulse')} | ||
| title="Take a tour" |
There was a problem hiding this comment.
When hasCompletedTour is true, this button becomes icon-only (no visible text). title is not a reliable accessible name for screen readers. Add an aria-label (or include a visually-hidden text node) so the trigger remains accessible in the completed state.
| title="Take a tour" | |
| title="Take a tour" | |
| aria-label="Take the tour" |
| vi.mock('../../../lib/demoMode', () => ({ | ||
| isDemoMode: () => true, getDemoMode: () => true, isNetlifyDeployment: false, | ||
| isDemoModeForced: false, canToggleDemoMode: () => true, setDemoMode: vi.fn(), | ||
| toggleDemoMode: vi.fn(), subscribeDemoMode: () => () => {}, | ||
| isDemoToken: () => true, hasRealToken: () => false, setDemoToken: vi.fn(), | ||
| isFeatureEnabled: () => true, | ||
| })) | ||
|
|
||
| vi.mock('../../../hooks/useDemoMode', () => ({ | ||
| getDemoMode: () => true, default: () => true, | ||
| useDemoMode: () => ({ isDemoMode: true, toggleDemoMode: vi.fn(), setDemoMode: vi.fn() }), | ||
| hasRealToken: () => false, isDemoModeForced: false, isNetlifyDeployment: false, | ||
| canToggleDemoMode: () => true, isDemoToken: () => true, setDemoToken: vi.fn(), | ||
| setGlobalDemoMode: vi.fn(), | ||
| })) | ||
|
|
||
| vi.mock('../../../lib/analytics', () => ({ | ||
| emitNavigate: vi.fn(), emitLogin: vi.fn(), emitEvent: vi.fn(), analyticsReady: Promise.resolve(), | ||
| emitAddCardModalOpened: vi.fn(), emitCardExpanded: vi.fn(), emitCardRefreshed: vi.fn(), | ||
| emitLinkedInShare: vi.fn(), | ||
| })) | ||
|
|
||
| vi.mock('../../../hooks/useTokenUsage', () => ({ | ||
| useTokenUsage: () => ({ usage: { total: 0, remaining: 0, used: 0 }, isLoading: false }), | ||
| tokenUsageTracker: { getUsage: () => ({ total: 0, remaining: 0, used: 0 }), trackRequest: vi.fn(), getSettings: () => ({ enabled: false }) }, | ||
| })) |
There was a problem hiding this comment.
This same mock setup (demo mode, analytics, token usage, i18n) is duplicated across many new smoke tests. To reduce upkeep and avoid widespread breakage when any of these modules change, consider extracting these into a shared test helper or a common Vitest setup file and reusing them across the test suite.
| describe('GitHubInviteButton', () => { | ||
| it('renders without crashing', () => { | ||
| const { container } = render(<GitHubInviteButton onClick={vi.fn()} />) | ||
| expect(container).toBeTruthy() |
There was a problem hiding this comment.
expect(container).toBeTruthy() is effectively a no-op because container will always be truthy if render() returns. If the intent is a smoke test, consider asserting something observable (e.g., getByRole('button')), or at least expect(container.firstChild).not.toBeNull() to ensure something actually rendered.
| expect(container).toBeTruthy() | |
| expect(container.firstChild).not.toBeNull() |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 2 code suggestion(s) and 1 general comment(s). @copilot Please apply all of the following code review suggestions:
Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
- Tour.tsx: Add aria-label on icon-only tour button (#4172) - ConsoleOfflineDetectionCard: Hoist GPU_CLUSTER_EXHAUSTION_THRESHOLD to module scope, fix comments "80%+" → ">80%" (#4185) - coverage-hourly.yml: Replace always() with success check to avoid noisy merge-coverage failures when shards fail (#4185) Signed-off-by: Andrew Anderson <andy@clubanderson.com>
…ow (#4186) - Tour.tsx: Add aria-label on icon-only tour button (#4172) - ConsoleOfflineDetectionCard: Hoist GPU_CLUSTER_EXHAUSTION_THRESHOLD to module scope, fix comments "80%+" → ">80%" (#4185) - coverage-hourly.yml: Replace always() with success check to avoid noisy merge-coverage failures when shards fail (#4185) Signed-off-by: Andrew Anderson <andy@clubanderson.com>
) * 🐛 Address Copilot comments: tour a11y, GPU threshold, coverage workflow - Tour.tsx: Add aria-label on icon-only tour button (#4172) - ConsoleOfflineDetectionCard: Hoist GPU_CLUSTER_EXHAUSTION_THRESHOLD to module scope, fix comments "80%+" → ">80%" (#4185) - coverage-hourly.yml: Replace always() with success check to avoid noisy merge-coverage failures when shards fail (#4185) Signed-off-by: Andrew Anderson <andy@clubanderson.com> * 🌱 Add debug logging for screenshot upload flow in feedback dialog All logs use console.debug with [Screenshot] prefix for easy filtering in browser DevTools. Logs at each stage: - File selection (drag, drop, paste, file picker): file count, types, sizes - FileReader: loaded data URI length, errors - Paste: clipboard item types, getAsFile results - Submit: screenshot count, sizes, timeout, success/failure details This helps team members diagnose why screenshot uploads fail on their machines while working on the reporter's machine. Signed-off-by: Andrew Anderson <andy@clubanderson.com> --------- Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Summary
<button>elements inTour.tsx(4 buttons) andOnboarding.tsx(4 buttons) with the sharedButtoncomponent for consistent styling and behaviorTest plan
npx tsc --noEmitpassesnpx vitest run)Closes #4164
Closes #4168