Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,11 @@ jobs:
- name: Lint
run: pnpm lint

- name: Knip
run: pnpm knip

- name: Test (API 100% line coverage gate)
run: pnpm test
run: pnpm test:coverage

Comment thread
mcalthrop marked this conversation as resolved.
- name: Generate OpenAPI code
run: pnpm openapi:generate
Expand Down
2 changes: 1 addition & 1 deletion PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Tasks and subtasks for building the bread-recipes app (SolidJS + Python REST + O
- [x] **4.5** Component library: evaluate options for SolidJS (e.g. **shadcn-solid** with Tailwind vs smaller stacks); record the decision; add the chosen tooling and migrate or adopt components on at least one real screen so the pattern is established.
- [x] **4.6** Recipe page: full recipe content and larger image; deep-linkable route (e.g. by id).
- [x] **4.7** **shadcn-solid adoption (full):** migrate remaining UI (app shell, home, recipe cards and detail sections, and any shared layout) to registry components and Tailwind utilities where it replaces bespoke CSS; align tokens with **`COMPONENT_LIBRARY.md`**; no orphaned hand-rolled controls that duplicate registry patterns. Update **`COMPONENT_LIBRARY.md`** when scope is complete.
- [ ] **4.8** MSW for tests; knip configured; Vitest coverage at 100% with a CI gate.
- [x] **4.8** MSW for tests; knip configured; Vitest coverage at 100% with a CI gate.

## 5. Contract testing (Pact)

Expand Down
6 changes: 3 additions & 3 deletions apps/api/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ From **`apps/api`** with dev dependencies installed (**`pip install -e ".[dev]"`
pnpm test
```

This runs **`pytest`** with **line coverage** for the **`app`** package, **fails under 100%**, and **omits** generated **`app/openapi/generated/`** (codegen output). To run tests without coverage (faster iteration):
This runs **`pytest`** without coverage for faster iteration.

```bash
.venv/bin/python -m pytest --no-cov
pnpm test:coverage
```

Configuration lives in **`pyproject.toml`** (**`[tool.pytest.ini_options]`**, **`[tool.coverage.*]`**).
The coverage variant runs **`pytest`** with **line coverage** for the **`app`** package, **fails under 100%**, and **omits** generated **`app/openapi/generated/`** (codegen output). Configuration lives in **`pyproject.toml`** (**`[tool.pytest.ini_options]`**, **`[tool.coverage.*]`**).

## Import order (Ruff / isort)

Expand Down
1 change: 1 addition & 0 deletions apps/api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"lint": "pnpm build && .venv/bin/ruff check app",
"lint:fix": ".venv/bin/ruff check app --fix",
"test": ".venv/bin/python -m pytest -v",
"test:coverage": "pnpm test --cov=app --cov-report=term-missing",
"postinstall": "bash -e -c 'test -d .venv || python3 -m venv .venv; .venv/bin/python -m pip install -e \".[dev]\"'"
}
}
2 changes: 1 addition & 1 deletion apps/api/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ exclude = ["**/app/**/*_test.py"]
testpaths = ["app"]
pythonpath = ["."]
python_files = ["*_test.py"]
addopts = "-q --cov=app --cov-report=term-missing"
addopts = "-q"

[tool.coverage.run]
relative_files = true
Expand Down
1 change: 1 addition & 0 deletions apps/web/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ lerna-debug.log*
node_modules
dist
dist-ssr
coverage
*.local

# Editor directories and files
Expand Down
12 changes: 12 additions & 0 deletions apps/web/knip.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"$schema": "https://unpkg.com/knip@5/schema.json",
"entry": ["src/index.tsx"],
"project": ["src/**/*.{ts,tsx}"],
"ignore": [
"src/api/generated/**",
"**/*.test.{ts,tsx}",
"src/components/ui/**",
"src/api/index.ts"
],
"ignoreDependencies": ["tailwindcss", "tailwindcss-animate"]
}
5 changes: 5 additions & 0 deletions apps/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
"lint": "biome check . && node scripts/assert-no-function-declarations.mjs",
"lint:fix": "biome check --write .",
"test": "vitest run",
"test:coverage": "vitest run --coverage",
"test:watch": "vitest",
"knip": "knip",
"openapi:generate": "openapi-ts -f openapi-ts.config.ts",
"openapi:validate": "pnpm --filter @solid-pact/openapi run lint"
},
Expand All @@ -28,7 +30,10 @@
"@solidjs/testing-library": "0.8.10",
"@tailwindcss/vite": "4.2.2",
"@types/node": "24.12.0",
"@vitest/coverage-v8": "4.1.2",
"jsdom": "29.0.1",
"knip": "5.80.1",
"msw": "2.12.7",
"tailwindcss": "4.2.2",
"tailwindcss-animate": "1.0.7",
"typescript": "5.9.3",
Expand Down
4 changes: 2 additions & 2 deletions apps/web/scripts/assert-no-function-declarations.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import fs from 'node:fs';
import path from 'node:path';

const rootDir = path.join(import.meta.dirname, '..');
/** Top-level / named `function` declarations (not `typeof x === 'function'`). */
/** Top-level / named `function` declarations (not `typeof x === 'function'` or `functions:`). */
const declarationLine =
/^\s*(?:export\s+(?:default\s+)?(?:async\s+)?|async\s+)?function\s*\*?\s*(?:[$\w]|\()/;
/^\s*(?:export\s+(?:default\s+)?(?:async\s+)?|async\s+)?\bfunction\b\s*\*?\s*(?:[$\w]|\()/;

function walkTsFiles(dir, out) {
for (const ent of fs.readdirSync(dir, { withFileTypes: true })) {
Expand Down
47 changes: 37 additions & 10 deletions apps/web/src/App.test.tsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,47 @@
import { render, screen } from '@solidjs/testing-library';
import { describe, expect, it, vi } from 'vitest';
import { fireEvent, render, screen } from '@solidjs/testing-library';
import { HttpResponse, http } from 'msw';
import { describe, expect, it } from 'vitest';
import type { RecipeDetail, RecipeSummary } from '@/api';
import { API_BASE } from '@/test/msw/constants';
import { server } from '@/test/msw/server';
import { App } from './App';

vi.mock('@/api', () => ({
listRecipes: vi.fn().mockResolvedValue({
data: [],
error: undefined,
request: new Request('http://127.0.0.1:8000/recipes'),
response: new Response(),
}),
}));
const listItem: RecipeSummary = {
id: 'nav-loaf',
title: 'Navigation loaf',
summary: 'For routing test',
imageUrl: 'https://example.com/t.jpg',
};

const detail: RecipeDetail = {
...listItem,
imageUrlLarge: 'https://example.com/l.jpg',
ingredients: ['flour'],
steps: ['bake'],
};

describe('App', () => {
it('renders the main heading', () => {
render(() => <App />);
const heading = screen.getByRole('heading', { level: 1 });
expect(heading.textContent).toBe('Bread Recipes');
});

it('navigates from the home list to a recipe detail route', async () => {
server.use(
http.get(`${API_BASE}/recipes`, () => HttpResponse.json([listItem])),
http.get(`${API_BASE}/recipes/:recipeId`, ({ params }) => {
if (params.recipeId !== 'nav-loaf') {
return HttpResponse.json({ message: 'Not found' }, { status: 404 });
}
return HttpResponse.json(detail);
}),
);
render(() => <App />);
const link = await screen.findByRole('link', { name: /Navigation loaf/i });
fireEvent.click(link);
expect(
await screen.findByRole('heading', { level: 2, name: 'Navigation loaf' }),
).toBeTruthy();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Card, CardContent, CardHeader } from '@/components/ui/card';
import { Heading3Panel } from '@/components/ui/typography';
import { cn } from '@/lib/utils';

export type RecipeDetailPanelProps = ComponentProps<'section'> & {
type RecipeDetailPanelProps = ComponentProps<'section'> & {
headingId: string;
heading: string;
children: JSX.Element;
Expand Down
23 changes: 23 additions & 0 deletions apps/web/src/lib/format-fetch-error.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { describe, expect, it } from 'vitest';
import { formatFetchError } from './format-fetch-error';

describe('formatFetchError', () => {
it('returns the message of a plain Error', () => {
expect(formatFetchError(new Error('oops'))).toBe('oops');
});

it('unwraps Error.cause recursively', () => {
const inner = new Error('inner');
const outer = new Error('Unknown error', { cause: inner });
expect(formatFetchError(outer)).toBe('inner');
});

it('reads message from a plain object with message', () => {
expect(formatFetchError({ message: 'from body' })).toBe('from body');
});

it('stringifies other values', () => {
expect(formatFetchError(null)).toBe('null');
expect(formatFetchError(42)).toBe('42');
});
});
21 changes: 21 additions & 0 deletions apps/web/src/lib/format-fetch-error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* Format errors for UI. Solid's `createResource` wraps non-`Error` rejections in
* `Error("Unknown error", { cause })` — unwrap `cause` for API `{ message }` bodies.
*/
export const formatFetchError = (err: unknown): string => {
if (err instanceof Error) {
if (err.cause !== undefined) {
return formatFetchError(err.cause);
}
return err.message;
}
if (
typeof err === 'object' &&
err !== null &&
'message' in err &&
typeof (err as { message: unknown }).message === 'string'
) {
return (err as { message: string }).message;
}
return String(err);
};
78 changes: 40 additions & 38 deletions apps/web/src/pages/Home.test.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { MemoryRouter, Route } from '@solidjs/router';
import { render, screen } from '@solidjs/testing-library';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { HttpResponse, http } from 'msw';
import { describe, expect, it } from 'vitest';
import type { RecipeSummary } from '@/api';
import { listRecipes } from '@/api';
import { API_BASE } from '@/test/msw/constants';
import { server } from '@/test/msw/server';
import { Home } from './Home';

const renderHome = (): ReturnType<typeof render> =>
Expand All @@ -12,58 +14,58 @@ const renderHome = (): ReturnType<typeof render> =>
</MemoryRouter>
));

vi.mock('@/api', async (importOriginal) => {
const mod = await importOriginal<typeof import('@/api')>();
return {
...mod,
listRecipes: vi.fn(),
};
});

const listOk = (
data: RecipeSummary[],
): {
data: RecipeSummary[];
error: undefined;
request: Request;
response: Response;
} => ({
data,
error: undefined,
request: new Request('http://127.0.0.1:8000/recipes'),
response: new Response(),
});

describe('Home', () => {
beforeEach(() => {
vi.mocked(listRecipes).mockReset();
it('shows a loading state while recipes are fetched', async () => {
const gate = Promise.withResolvers<void>();
server.use(
http.get(`${API_BASE}/recipes`, async () => {
await gate.promise;
return HttpResponse.json([]);
}),
);
renderHome();
expect(await screen.findByText('Loading recipes…')).toBeTruthy();
gate.resolve();
expect(await screen.findByText(/No recipes to show yet/i)).toBeTruthy();
});

it('treats a null API payload as an empty recipe list', async () => {
server.use(http.get(`${API_BASE}/recipes`, () => HttpResponse.json(null)));
renderHome();
expect(await screen.findByText(/No recipes to show yet/i)).toBeTruthy();
});

it('shows empty state when the API returns no recipes', async () => {
vi.mocked(listRecipes).mockResolvedValue(listOk([]));
server.use(http.get(`${API_BASE}/recipes`, () => HttpResponse.json([])));
renderHome();
expect(await screen.findByText(/No recipes to show yet/i)).toBeTruthy();
});

it('renders linked recipe cards when the API returns recipes', async () => {
vi.mocked(listRecipes).mockResolvedValue(
listOk([
{
id: 'sourdough-1',
title: 'Seeded sourdough',
summary: 'Overnight ferment',
imageUrl: 'https://example.com/thumb.jpg',
},
]),
server.use(
http.get(`${API_BASE}/recipes`, () =>
HttpResponse.json<RecipeSummary[]>([
{
id: 'sourdough-1',
title: 'Seeded sourdough',
summary: 'Overnight ferment',
imageUrl: 'https://example.com/thumb.jpg',
},
]),
),
);
renderHome();
expect(await screen.findByText('Seeded sourdough')).toBeTruthy();
const link = screen.getByRole('link', { name: /Seeded sourdough/i });
expect(link.getAttribute('href')).toBe('/recipes/sourdough-1');
});

it('shows an alert when the loader throws', async () => {
vi.mocked(listRecipes).mockRejectedValue(new Error('Bad gateway'));
it('shows an alert when the API returns an error', async () => {
server.use(
http.get(`${API_BASE}/recipes`, () =>
HttpResponse.json({ message: 'Bad gateway' }, { status: 502 }),
),
);
renderHome();
const alert = await screen.findByRole('alert');
expect(alert.textContent).toContain('Bad gateway');
Expand Down
5 changes: 2 additions & 3 deletions apps/web/src/pages/Home.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { Alert, AlertDescription } from '@/components/ui/alert';
import { LoadingRegion } from '@/components/ui/loading-region';
import { Skeleton } from '@/components/ui/skeleton';
import { Heading2, TextLede, TextMuted } from '@/components/ui/typography';
import { formatFetchError } from '@/lib/format-fetch-error';

export const Home = (): JSX.Element => {
const [recipes] = createResource(() =>
Expand Down Expand Up @@ -35,9 +36,7 @@ export const Home = (): JSX.Element => {
<Show when={recipes.error} keyed>
{(err: unknown) => (
<Alert variant="destructive">
<AlertDescription>
{err instanceof Error ? err.message : String(err)}
</AlertDescription>
<AlertDescription>{formatFetchError(err)}</AlertDescription>
</Alert>
)}
</Show>
Expand Down
25 changes: 25 additions & 0 deletions apps/web/src/pages/RecipePage.missing-id.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { MemoryRouter, Route } from '@solidjs/router';
import { render, screen } from '@solidjs/testing-library';
import { describe, expect, it, vi } from 'vitest';
import { RecipePage } from './RecipePage';

vi.mock('@solidjs/router', async (importOriginal) => {
const mod = await importOriginal<typeof import('@solidjs/router')>();
return {
...mod,
/** Solid skips the fetcher when the source is `undefined`; an empty id still runs it. */
useParams: () => ({ id: '' }),
};
});

describe('RecipePage (missing id)', () => {
it('shows an error when the route param id is empty', async () => {
render(() => (
<MemoryRouter>
<Route path="/" component={RecipePage} />
</MemoryRouter>
));
const alert = await screen.findByRole('alert');
expect(alert.textContent).toContain('Missing recipe id');
});
});
Loading
Loading