test(SAL-3525): router and route wiring coverage by salahmyn · Pull Request #64 · SallaApp/theme-raed-react

salahmyn · 2026-10-26T21:04:20Z

Summary

First coverage PR for SAL-3525 (router and route wiring).

Tests for app/router.tsx: registry registrations, home block overrides, per-block placement config (class names for boxed/merged/repeated variants), getRouter singleton on the client vs fresh per request on the server.
Tests for every file in app/routes/: engine loader args, head, loading skeleton, rendered component, search validation for orders/wallet/wishlist, product referrer cleanup, page-single mapping, redirects.
Tests for client, hydrate, start, server, routes.ts and the dev/production branches of __root.tsx.
vitest.config.ts: floor raised to lines 88, branches 75, functions 83, statements 85 (measured 88.35 / 75.73 / 83.01 / 85.68, rounded down). app/router.tsx is now measured by v8 too.
routeTree.gen is aliased to a stub in vitest (tests/stubs/routeTree.gen.ts) because the generated file is absent on a clean checkout.

No application code changed. No coverage exclusions added.

Verification

build, typecheck, lint, format:check, test:coverage, check:manifest and knip exit 0 locally (Node 22).

Exclusions

app/routeTree.gen.ts: generated by the router plugin at build time (CI generates it before testing).

Confidence Score: 4/5

The test assertions and selector names must meet the repository’s requirements before merging.

Findings

Production editor check misses regressions ▶
Product remounting is not tested ▶
Cleanup order is not tested ▶
Selectors break the naming rule ▶

Fix with agent prompt

### Issue 1
tests/routes/root.test.tsx:85-91
This test can pass even if the settings editor loses its `DEV` guard. After `vi.resetModules()`, the editor would be a fresh lazy component. `renderToString()` renders its empty `Suspense` fallback without waiting, so the missing text does not prove the editor is disabled.

The repository requires assertions to check the claimed behavior. Before merging, wait for rendering to finish, as the development test does, then check that the editor is absent.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

### Issue 2
tests/routes/routeFiles.test.tsx:382-389
This test claims to check remounting, but renders only product 5 and checks that `ProductPage` exists. Removing `key={data.product.id}` would still pass.

The repository requires assertions to check the claimed behavior. Before merging, rerender with another product ID and use a stateful stub or mount counter to check that React creates a new instance.

### Issue 3
tests/routes/entry.test.tsx:59-61
This test checks that both functions run, but not that `removeEdgeInjections` runs before `hydrateRoot`. Reversing those calls would still pass despite the test's “then hydrates” claim.

The repository requires assertions to check the claimed behavior. Before merging, compare their recorded call order to protect cleanup before hydration.

### Issue 4
tests/routes/router.test.tsx:undefined-21
The new `data-testid` values `slider-skeleton` and `list-page` do not follow the required `<feature>-<component>-<action>` format. Before merging, rename both values and their matching queries to include all three parts.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Adds tests for router setup, route files, browser startup, server startup, and root rendering.

Route tests check each page's path, loader, head, and loading screen.
Account route tests check page and order filters reach the right loader.
Special route tests cover product, cart, page, and redirect wiring.
Router tests check how theme blocks replace engine blocks.
Home block tests check loading space and placement classes.
Router tests check browser reuse and fresh server instances.
Entry tests trace client and server startup hooks.
Root tests check development tools and production styles.
Vitest can run without a generated route tree and enforces higher coverage floors.

_{Reviews (2) · Last reviewed commit: "test(SAL-3525): exclude the generated ro..." · Reviewed by Greptile}

… floor Tests the router registry wiring, every route file (loaders, heads, skeletons, search validation), the client/server/start entries and the root shell. Floor raised to 88/75/83/85 (lines/branches/functions/statements). routeTree.gen is aliased to a stub under vitest since it is generated at build time. Made-with: agent Co-authored-by: mc-agents[bot] <298817976+mc-agents[bot]@users.noreply.github.com>

CI generates app/routeTree.gen.ts before testing; it is generated code and was counted at 0%. Made-with: agent Co-authored-by: mc-agents[bot] <298817976+mc-agents[bot]@users.noreply.github.com>

salahmyn · 2026-10-26T21:09:42Z

@greptile-apps please review

greptile-apps · 2026-10-26T21:13:11Z

+  it('remounts the product page for each product and shows the detail skeleton while loading', async () => {
+    const { state, stub } = await h;
+    state.loaderData = { product: { id: 5 } };
+    const route = await load('$slug.p$id');
+    expect(route.head).toEqual({ withHeadOf: stub('Product') });
+    const Component = route.component;
+    render(<Component />);
+    expect(document.querySelector('[data-stub="ProductPage"]')).not.toBeNull();


Product remounting is not tested

This test claims to check remounting, but renders only product 5 and checks that ProductPage exists. Removing key={data.product.id} would still pass.

The repository requires assertions to check the claimed behavior. Before merging, rerender with another product ID and use a stateful stub or mount counter to check that React creates a new instance.

Rule Used: What: Reject assertions that don't actually validate the expected behavior or outcome. Why: Fake assertions give false confidence in test coverage and fail to catch real bugs. Good: ```python def test_user_creation(): user = create_user("John",... (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: tests/routes/routeFiles.test.tsx Line: 382-389 Comment: **Product remounting is not tested** This test claims to check remounting, but renders only product 5 and checks that `ProductPage` exists. Removing `key={data.product.id}` would still pass. The repository requires assertions to check the claimed behavior. Before merging, rerender with another product ID and use a stateful stub or mount counter to check that React creates a new instance. **Rule Used:** What: Reject assertions that don't actually validate the expected behavior or outcome. Why: Fake assertions give false confidence in test coverage and fail to catch real bugs. Good: ```python def test_user_creation(): user = create_user("John",... ([source](https://app.greptile.com/salla/-/custom-context?memory=4475f6ab-b5f0-46e4-98d5-1f1eb133c1df)) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

greptile-apps · 2026-10-26T21:13:11Z

+    expect(e.removeEdgeInjections).toHaveBeenCalledTimes(1);
+    await vi.waitFor(() => expect(e.hydrateRoot).toHaveBeenCalledTimes(1));
+    expect(e.hydrateRoot.mock.calls[0][0]).toBe(document);


Cleanup order is not tested

This test checks that both functions run, but not that removeEdgeInjections runs before hydrateRoot. Reversing those calls would still pass despite the test's “then hydrates” claim.

The repository requires assertions to check the claimed behavior. Before merging, compare their recorded call order to protect cleanup before hydration.

Rule Used: What: Reject assertions that don't actually validate the expected behavior or outcome. Why: Fake assertions give false confidence in test coverage and fail to catch real bugs. Good: ```python def test_user_creation(): user = create_user("John",... (source)

Knowledge Base Used: Runtime entrypoints

Prompt To Fix With AI

This is a comment left during a code review. Path: tests/routes/entry.test.tsx Line: 59-61 Comment: **Cleanup order is not tested** This test checks that both functions run, but not that `removeEdgeInjections` runs before `hydrateRoot`. Reversing those calls would still pass despite the test's “then hydrates” claim. The repository requires assertions to check the claimed behavior. Before merging, compare their recorded call order to protect cleanup before hydration. **Rule Used:** What: Reject assertions that don't actually validate the expected behavior or outcome. Why: Fake assertions give false confidence in test coverage and fail to catch real bugs. Good: ```python def test_user_creation(): user = create_user("John",... ([source](https://app.greptile.com/salla/-/custom-context?memory=4475f6ab-b5f0-46e4-98d5-1f1eb133c1df)) **Knowledge Base Used:** [Runtime entrypoints](https://app.greptile.com/salla/-/custom-context/knowledge-base/sallaapp/theme-raed-react/-/docs/runtime-entrypoints.md) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

greptile-apps · 2026-10-26T21:13:11Z

+  DefaultHomeComponents: { engineOnly: 'engine', brands: 'engine-brands' },
+}));
+vi.mock('@salla.sa/twilight-components-react', () => ({
+  SliderSkeleton: () => <div data-testid="slider-skeleton" />,


Selectors break the naming rule

The new data-testid values slider-skeleton and list-page do not follow the required <feature>-<component>-<action> format. Before merging, rename both values and their matching queries to include all three parts.

Rule Used: # Code Review Rule: data-testid Naming Convention What: All data-testid attributes must follow the kebab-case format <feature>-<component>-<action>, with dynamic elements including their item ID. Why: Consistent selector naming enables ... (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: tests/routes/router.test.tsx Line: 21 Comment: **Selectors break the naming rule** The new `data-testid` values `slider-skeleton` and `list-page` do not follow the required `<feature>-<component>-<action>` format. Before merging, rename both values and their matching queries to include all three parts. **Rule Used:** # Code Review Rule: data-testid Naming Convention **What:** All `data-testid` attributes must follow the kebab-case format `<feature>-<component>-<action>`, with dynamic elements including their item ID. **Why:** Consistent selector naming enables ... ([source](https://app.greptile.com/salla/-/custom-context?memory=43bb0927-6c63-4b85-9b79-52904ec74d75)) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

salahmyn · 2026-10-27T07:07:40Z

@greptileai review

greptile-apps · 2026-10-27T07:11:04Z

+      const html = renderToString(<Shell />);
+
+      expect(rendered.stylesheet?.critical).toEqual({
+        [HOME_ROUTE_ID]: '',
+        [PRODUCT_ROUTE_ID]: '',
+      });
+      expect(html).not.toContain('dev-settings-widget');


Production editor check misses regressions

This test can pass even if the settings editor loses its DEV guard. After vi.resetModules(), the editor would be a fresh lazy component. renderToString() renders its empty Suspense fallback without waiting, so the missing text does not prove the editor is disabled.

The repository requires assertions to check the claimed behavior. Before merging, wait for rendering to finish, as the development test does, then check that the editor is absent.

Rule Used: What: Reject assertions that don't actually validate the expected behavior or outcome. Why: Fake assertions give false confidence in test coverage and fail to catch real bugs. Good: ```python def test_user_creation(): user = create_user("John",... (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: tests/routes/root.test.tsx Line: 85-91 Comment: **Production editor check misses regressions** This test can pass even if the settings editor loses its `DEV` guard. After `vi.resetModules()`, the editor would be a fresh lazy component. `renderToString()` renders its empty `Suspense` fallback without waiting, so the missing text does not prove the editor is disabled. The repository requires assertions to check the claimed behavior. Before merging, wait for rendering to finish, as the development test does, then check that the editor is absent. **Rule Used:** What: Reject assertions that don't actually validate the expected behavior or outcome. Why: Fake assertions give false confidence in test coverage and fail to catch real bugs. Good: ```python def test_user_creation(): user = create_user("John",... ([source](https://app.greptile.com/salla/-/custom-context?memory=4475f6ab-b5f0-46e4-98d5-1f1eb133c1df)) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

… is absent in production Made-with: agent Co-authored-by: mc-agents[bot] <298817976+mc-agents[bot]@users.noreply.github.com>

greptile-apps · 2026-10-27T10:51:12Z

Automatic reviews are disabled for this repository.

sallainternalbot Bot marked this pull request as draft October 26, 2026 21:04

test(SAL-3525): exclude the generated route tree from coverage

1bc1d0a

CI generates app/routeTree.gen.ts before testing; it is generated code and was counted at 0%. Made-with: agent Co-authored-by: mc-agents[bot] <298817976+mc-agents[bot]@users.noreply.github.com>

greptile-apps Bot reviewed Oct 26, 2026

View reviewed changes

greptile-apps Bot reviewed Oct 27, 2026

View reviewed changes

test(SAL-3525): wait for the lazy settings editor before asserting it…

9957848

… is absent in production Made-with: agent Co-authored-by: mc-agents[bot] <298817976+mc-agents[bot]@users.noreply.github.com>

salahmyn marked this pull request as ready for review October 27, 2026 10:51

salahmyn merged commit 82943f5 into feature/quality-gates-twig-parity-audit Oct 27, 2026
2 of 3 checks passed

salahmyn deleted the test/SAL-3525-router-and-registry-coverage branch October 27, 2026 13:42

Navigation Menu

Repository navigation

test(SAL-3525): router and route wiring coverage - #64

test(SAL-3525): router and route wiring coverage#64
salahmyn merged 3 commits into
feature/quality-gates-twig-parity-auditfrom
test/SAL-3525-router-and-registry-coverage

salahmyn commented Oct 26, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

salahmyn commented Oct 26, 2026

Uh oh!

greptile-apps Bot Oct 26, 2026

Uh oh!

greptile-apps Bot Oct 26, 2026

Uh oh!

greptile-apps Bot Oct 26, 2026

Uh oh!

salahmyn commented Oct 27, 2026

Uh oh!

greptile-apps Bot Oct 27, 2026

Uh oh!

greptile-apps Bot commented Oct 27, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

salahmyn commented Oct 26, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Exclusions

Confidence Score: 4/5

Findings

Summary

Uh oh!

salahmyn commented Oct 26, 2026

Uh oh!

greptile-apps Bot Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

salahmyn commented Oct 27, 2026

Uh oh!

greptile-apps Bot Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented Oct 27, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

salahmyn commented Oct 26, 2026 •

edited by greptile-apps Bot

Loading