Skip to content

Commit 35a2583

Browse files
committed
ci+test: wire vitest into CI + add tests for HelpTooltip + install widget
The frontend test setup (vitest + happy-dom + @testing-library/react + 6 existing test files = 37 passing tests) was already wired up but NOT running in CI — deploy.yml only invoked ``npm audit`` and ``npm run build``, so the existing tests were gating exactly nothing. A regression any of them would catch could ship to prod undetected. Three changes in one commit: 1. Add ``npm test`` to the frontend CI job ───────────────────────────────────────── New "Run frontend tests (vitest)" step runs BEFORE the build so a test-caught regression doesn't get the chance to ship via a successful build. ~10s on CI hardware, no caching needed. 2. New tests/components/HelpTooltip.test.jsx (9 tests) ────────────────────────────────────────────────── The reusable "?" popover shipped in the contextual-help work. Covers: hidden until open, click-toggle, Escape-to-close, click-outside-to-close, optional Learn-more link, custom aria-label, aria-expanded sync. Caught a real interaction bug while writing: the original component had ``onMouseEnter={open}`` + ``onFocus={open}`` + ``onClick={toggle}`` — userEvent.click synthesises mouseenter → focus → click in sequence, so a single click on a closed tooltip would open via mouseenter, then immediately re-close via the click toggle. Same bug fires on touch devices where there's no real "hover" before the tap. Fixed by simplifying the trigger to click-toggle only. Hover no longer auto-opens — keyboard accessibility is preserved because Enter/Space on a focused button fires click. 3. New tests/components/InstallCloudNodeCard.test.jsx (6 tests) ─────────────────────────────────────────────────────────── The single highest-stakes UX flow in the app: just signed up, empty grid, customer needs to run a CLI command. A regression in this widget = silent loss of every new sign-up. Mocks @clerk/clerk-react useAuth + the createNode service helper so tests don't need a Clerk provider or a real fetch. Covers: - Initial state shows CTA only, no credentials revealed - Does NOT auto-create a node on mount (orphan-node guard) - Click → calls createNode + transitions to the tabbed view - The displayed Linux/macOS one-liner contains the returned credentials (--node-id + --key) — single most-important assertion in the file - All three OS tabs render after creds load - Switching to Windows tab shows the MSI download link - API failure surfaces as inline error so the user can retry Frontend test count went 37 → 52 (+15). Backend untouched, still 538. ruff clean. Build succeeds.
1 parent 33ed54d commit 35a2583

4 files changed

Lines changed: 339 additions & 9 deletions

File tree

.github/workflows/deploy.yml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,21 @@ jobs:
7878
working-directory: frontend
7979
run: npm audit --audit-level=high --omit=dev
8080

81+
# Vitest component tests — run BEFORE the build so a regression
82+
# caught by tests doesn't get the chance to ship via a successful
83+
# build. We have ~50 tests today (HelpTooltip, InstallCloudNodeCard,
84+
# UpgradeModal, EmptyState, the API service helpers, the docs
85+
# page, and a sanity smoke test); the suite runs in <10s on CI
86+
# so the speed cost is negligible.
87+
#
88+
# ``npm test`` is the package.json alias for ``vitest run``
89+
# (one-shot, exits with status, no watch). Vitest auto-discovers
90+
# files matching the ``include: ["tests/**/*.test.{js,jsx}"]``
91+
# pattern in vite.config.js.
92+
- name: Run frontend tests (vitest)
93+
working-directory: frontend
94+
run: npm test
95+
8196
# Build now so a syntax or type error fails CI here rather than
8297
# mid-deploy. Catches the same class of bug as backend pytest.
8398
- name: Build production bundle

frontend/src/components/HelpTooltip.jsx

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,14 @@ export default function HelpTooltip({ children, docHref, label = "Help" }) {
6363
<button
6464
type="button"
6565
className="help-tooltip-trigger"
66+
// Click-toggle only. Avoids a subtle interaction bug where
67+
// hover-to-open + click-to-toggle would race on touch devices
68+
// (the synthesized mouseenter fires immediately before the
69+
// click, opening then immediately re-closing the popover on
70+
// a single tap). Keyboard users activate via Enter/Space
71+
// which fires click, so they get the same toggle behaviour
72+
// — accessibility preserved.
6673
onClick={() => setOpen((v) => !v)}
67-
onMouseEnter={() => setOpen(true)}
68-
onFocus={() => setOpen(true)}
69-
// Don't blur-close — losing focus to the popover content
70-
// (e.g. clicking the "Learn more" link) shouldn't dismiss
71-
// the popover before the click registers.
7274
aria-label={label}
7375
aria-expanded={open}
7476
aria-controls={popoverId}
@@ -80,10 +82,6 @@ export default function HelpTooltip({ children, docHref, label = "Help" }) {
8082
id={popoverId}
8183
className="help-tooltip-popover"
8284
role="tooltip"
83-
// Mouse-leave on the popover wrapper closes — the trigger
84-
// also gets re-rendered with onMouseEnter so a tight back-
85-
// and-forth between trigger and popover stays open.
86-
onMouseLeave={() => setOpen(false)}
8785
>
8886
<div className="help-tooltip-body">{children}</div>
8987
{docHref && (
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
// Smoke tests for HelpTooltip — the reusable "?" icon + popover used
2+
// next to confusing setting labels (recording policy, MCP scope picker,
3+
// motion-email default-OFF rationale).
4+
//
5+
// What we're pinning here:
6+
//
7+
// - The popover is hidden until the user opens it (avoids
8+
// accidentally surfacing copy on every render).
9+
// - Click toggles open AND close — important because hover-only
10+
// popovers don't work on touch devices.
11+
// - Escape and click-outside close the popover (both are universal
12+
// dismissal patterns; both must work or the popover sticks open
13+
// and obscures the page).
14+
// - The optional ``docHref`` renders a "Learn more →" link only
15+
// when present (caller-controlled).
16+
// - Accessibility: the trigger gets aria-expanded that flips
17+
// true/false in sync with the popover's visibility.
18+
19+
import { describe, it, expect } from 'vitest'
20+
import { render, screen } from '@testing-library/react'
21+
import userEvent from '@testing-library/user-event'
22+
23+
import HelpTooltip from '../../src/components/HelpTooltip.jsx'
24+
25+
describe('HelpTooltip', () => {
26+
it('does not render the popover until the user opens it', () => {
27+
render(<HelpTooltip>helpful info</HelpTooltip>)
28+
29+
// The trigger is always visible.
30+
expect(screen.getByRole('button')).toBeInTheDocument()
31+
// The body copy is gated behind the open state.
32+
expect(screen.queryByRole('tooltip')).not.toBeInTheDocument()
33+
expect(screen.queryByText('helpful info')).not.toBeInTheDocument()
34+
})
35+
36+
it('opens on click and shows the body copy', async () => {
37+
const user = userEvent.setup()
38+
render(<HelpTooltip>helpful info</HelpTooltip>)
39+
40+
await user.click(screen.getByRole('button'))
41+
42+
expect(screen.getByRole('tooltip')).toBeInTheDocument()
43+
expect(screen.getByText('helpful info')).toBeInTheDocument()
44+
})
45+
46+
it('toggles closed on a second click', async () => {
47+
const user = userEvent.setup()
48+
render(<HelpTooltip>helpful info</HelpTooltip>)
49+
50+
const trigger = screen.getByRole('button')
51+
await user.click(trigger)
52+
expect(screen.getByRole('tooltip')).toBeInTheDocument()
53+
54+
await user.click(trigger)
55+
expect(screen.queryByRole('tooltip')).not.toBeInTheDocument()
56+
})
57+
58+
it('closes on Escape', async () => {
59+
const user = userEvent.setup()
60+
render(<HelpTooltip>helpful info</HelpTooltip>)
61+
62+
await user.click(screen.getByRole('button'))
63+
expect(screen.getByRole('tooltip')).toBeInTheDocument()
64+
65+
await user.keyboard('{Escape}')
66+
expect(screen.queryByRole('tooltip')).not.toBeInTheDocument()
67+
})
68+
69+
it('closes on click-outside', async () => {
70+
const user = userEvent.setup()
71+
render(
72+
<div>
73+
<HelpTooltip>helpful info</HelpTooltip>
74+
<button type="button">elsewhere</button>
75+
</div>,
76+
)
77+
78+
// Open the tooltip by name so the click-outside button doesn't
79+
// get matched.
80+
await user.click(screen.getByRole('button', { name: /help/i }))
81+
expect(screen.getByRole('tooltip')).toBeInTheDocument()
82+
83+
// Click a sibling button → tooltip closes.
84+
await user.click(screen.getByText('elsewhere'))
85+
expect(screen.queryByRole('tooltip')).not.toBeInTheDocument()
86+
})
87+
88+
it('renders a "Learn more" link when docHref is provided', async () => {
89+
const user = userEvent.setup()
90+
render(
91+
<HelpTooltip docHref="https://example.test/docs">
92+
gist
93+
</HelpTooltip>,
94+
)
95+
96+
await user.click(screen.getByRole('button'))
97+
98+
const link = screen.getByRole('link', { name: /learn more/i })
99+
expect(link).toBeInTheDocument()
100+
expect(link).toHaveAttribute('href', 'https://example.test/docs')
101+
})
102+
103+
it('omits the "Learn more" link when docHref is not provided', async () => {
104+
const user = userEvent.setup()
105+
render(<HelpTooltip>just gist</HelpTooltip>)
106+
107+
await user.click(screen.getByRole('button'))
108+
109+
expect(screen.queryByRole('link', { name: /learn more/i })).not.toBeInTheDocument()
110+
})
111+
112+
it('uses the provided aria-label on the trigger', () => {
113+
render(
114+
<HelpTooltip label="Help: recording policy">explanation</HelpTooltip>,
115+
)
116+
117+
expect(
118+
screen.getByRole('button', { name: 'Help: recording policy' }),
119+
).toBeInTheDocument()
120+
})
121+
122+
it('flips aria-expanded as the popover opens and closes', async () => {
123+
const user = userEvent.setup()
124+
render(<HelpTooltip>info</HelpTooltip>)
125+
126+
const trigger = screen.getByRole('button')
127+
expect(trigger).toHaveAttribute('aria-expanded', 'false')
128+
129+
await user.click(trigger)
130+
expect(trigger).toHaveAttribute('aria-expanded', 'true')
131+
132+
await user.click(trigger)
133+
expect(trigger).toHaveAttribute('aria-expanded', 'false')
134+
})
135+
})
Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
// Smoke tests for InstallCloudNodeCard — the in-app CloudNode install
2+
// widget that replaced the "click out to GitHub README" flow on the
3+
// empty-state dashboard.
4+
//
5+
// The single biggest moment of customer drop-off lives in this
6+
// component: just signed up, empty grid, need to run a CLI command.
7+
// A regression that broke the click → POST /api/nodes → display
8+
// credentialed one-liner flow would silently lose every new sign-up.
9+
//
10+
// What's pinned:
11+
//
12+
// - Initial state shows the "Get my install command →" CTA with no
13+
// credentials revealed (catches the orphan-node bug — auto-creating
14+
// on mount would spam the org with unused nodes).
15+
// - Click → calls createNode + transitions to the tabbed install view.
16+
// - The displayed Linux/macOS one-liner contains the returned
17+
// credentials (node_id + api_key). This is the single most-
18+
// important assertion — without these in the command, the install
19+
// wouldn't work end-to-end.
20+
// - OS tabs render and the user can switch between them.
21+
// - API failure surfaces as an inline error (e.g. plan limit hit
22+
// would otherwise silently leave the button disabled).
23+
24+
import { describe, it, expect, vi, beforeEach } from 'vitest'
25+
import { render, screen, waitFor } from '@testing-library/react'
26+
import userEvent from '@testing-library/user-event'
27+
28+
// Mock Clerk's useAuth so the component doesn't need a ClerkProvider
29+
// wrapper. The widget's only Clerk usage is `getToken()`, which
30+
// passes through to our createNode service helper — we mock that
31+
// helper too.
32+
vi.mock('@clerk/clerk-react', () => ({
33+
useAuth: () => ({
34+
getToken: () => Promise.resolve('test-jwt'),
35+
}),
36+
}))
37+
38+
// Mock the API helper so tests don't actually fetch. Override per
39+
// test by reassigning the .mockResolvedValueOnce / .mockRejectedValueOnce
40+
// stub.
41+
const mockCreateNode = vi.fn()
42+
vi.mock('../../src/services/api', () => ({
43+
createNode: (...args) => mockCreateNode(...args),
44+
}))
45+
46+
import InstallCloudNodeCard from '../../src/components/InstallCloudNodeCard.jsx'
47+
48+
49+
beforeEach(() => {
50+
mockCreateNode.mockReset()
51+
})
52+
53+
54+
describe('InstallCloudNodeCard', () => {
55+
it('renders the initial CTA without revealing credentials', () => {
56+
render(<InstallCloudNodeCard />)
57+
58+
expect(
59+
screen.getByRole('button', { name: /Get my install command/i }),
60+
).toBeInTheDocument()
61+
// No credential or command bytes leak before the user clicks.
62+
expect(screen.queryByText(/install\.sh/)).not.toBeInTheDocument()
63+
expect(screen.queryByText(/--node-id/)).not.toBeInTheDocument()
64+
})
65+
66+
it('does NOT auto-create a node on mount (orphan-node guard)', () => {
67+
// The widget previously considered auto-creating but explicitly
68+
// doesn't — a hot reload during dev or a user reloading the
69+
// dashboard repeatedly while troubleshooting their network
70+
// would otherwise burn through the plan's max_nodes limit.
71+
// Pin "no API call without an explicit click".
72+
render(<InstallCloudNodeCard />)
73+
expect(mockCreateNode).not.toHaveBeenCalled()
74+
})
75+
76+
it('calls createNode and reveals the credentialed one-liner on click', async () => {
77+
mockCreateNode.mockResolvedValueOnce({
78+
node_id: 'abc123',
79+
api_key: 'secret-key-xyz',
80+
})
81+
const user = userEvent.setup()
82+
render(<InstallCloudNodeCard />)
83+
84+
await user.click(
85+
screen.getByRole('button', { name: /Get my install command/i }),
86+
)
87+
88+
// createNode invoked exactly once with our auto-name.
89+
await waitFor(() => expect(mockCreateNode).toHaveBeenCalledTimes(1))
90+
expect(mockCreateNode).toHaveBeenCalledWith(
91+
expect.any(Function), // getToken
92+
'First CloudNode',
93+
)
94+
95+
// Wait for the post-creds state to render.
96+
await waitFor(() =>
97+
expect(screen.getByRole('tablist')).toBeInTheDocument(),
98+
)
99+
100+
// The displayed command MUST contain the returned credentials —
101+
// without these, the install command is useless. This is the
102+
// single highest-impact assertion in the file.
103+
const codeEl = document.querySelector('.install-command code')
104+
expect(codeEl).toBeInTheDocument()
105+
expect(codeEl.textContent).toContain('--node-id abc123')
106+
expect(codeEl.textContent).toContain('--key secret-key-xyz')
107+
})
108+
109+
it('renders all three OS tabs after credentials are loaded', async () => {
110+
mockCreateNode.mockResolvedValueOnce({
111+
node_id: 'n1', api_key: 'k1',
112+
})
113+
const user = userEvent.setup()
114+
render(<InstallCloudNodeCard />)
115+
116+
await user.click(
117+
screen.getByRole('button', { name: /Get my install command/i }),
118+
)
119+
await waitFor(() =>
120+
expect(screen.getByRole('tablist')).toBeInTheDocument(),
121+
)
122+
123+
// All three tabs render — Linux, macOS, Windows. Catches a
124+
// regression where someone simplified the tab loop and dropped one.
125+
expect(screen.getByRole('tab', { name: /Linux/i })).toBeInTheDocument()
126+
expect(screen.getByRole('tab', { name: /macOS/i })).toBeInTheDocument()
127+
expect(screen.getByRole('tab', { name: /Windows/i })).toBeInTheDocument()
128+
})
129+
130+
it('switches to the Windows tab and shows the MSI download path', async () => {
131+
mockCreateNode.mockResolvedValueOnce({
132+
node_id: 'n1', api_key: 'k1',
133+
})
134+
const user = userEvent.setup()
135+
render(<InstallCloudNodeCard />)
136+
137+
await user.click(
138+
screen.getByRole('button', { name: /Get my install command/i }),
139+
)
140+
await waitFor(() =>
141+
expect(screen.getByRole('tab', { name: /Windows/i })).toBeInTheDocument(),
142+
)
143+
144+
await user.click(screen.getByRole('tab', { name: /Windows/i }))
145+
146+
// Windows tab uses an MSI download (different from Linux/macOS
147+
// curl-pipe-bash). The download anchor must point at our
148+
// backend's redirect endpoint.
149+
const downloadLink = screen.getByRole('link', {
150+
name: /Download CloudNode for Windows/i,
151+
})
152+
expect(downloadLink).toBeInTheDocument()
153+
expect(downloadLink.getAttribute('href')).toMatch(
154+
/\/downloads\/windows\/x64$/,
155+
)
156+
})
157+
158+
it('surfaces API errors inline so the user knows what went wrong', async () => {
159+
// Simulate "Node limit reached on the Free plan" or similar.
160+
mockCreateNode.mockRejectedValueOnce(
161+
new Error('Node limit reached (3 on Free plan). Upgrade your plan.'),
162+
)
163+
const user = userEvent.setup()
164+
render(<InstallCloudNodeCard />)
165+
166+
await user.click(
167+
screen.getByRole('button', { name: /Get my install command/i }),
168+
)
169+
170+
await waitFor(() =>
171+
expect(screen.getByRole('alert')).toBeInTheDocument(),
172+
)
173+
expect(screen.getByRole('alert')).toHaveTextContent(/Node limit reached/)
174+
175+
// Still in the initial state (CTA visible, tabs not rendered)
176+
// so the user can retry.
177+
expect(
178+
screen.getByRole('button', { name: /Get my install command/i }),
179+
).toBeInTheDocument()
180+
expect(screen.queryByRole('tablist')).not.toBeInTheDocument()
181+
})
182+
})

0 commit comments

Comments
 (0)