Skip to content

Commit 3e0975b

Browse files
JSv4JSv4
committed
Update user-profile CTs for the dumb-consumer UserProfileRoute
The hook-ordering regression test (issue #1295) was guarding a bug that can no longer recur: useQuery is no longer in UserProfileRoute at all, since the GET_USER call moved to CentralRouteManager. Both that test and the query-driven loading/error CTs now point at non-existent behavior. Replaced the obsolete describe blocks with state-driven tests that mount the new dumb consumer behind small wrappers (UserProfileRouteTestWrappers) that seed openedUser / routeLoading / routeError in the browser context. Playwright CT mounts run in the page, so test.beforeEach (which executes in node) cannot reach the makeVar instances the component reads — the wrappers solve that by setting the vars synchronously in render before the child mounts. The view-level "should render public user profile" screenshot test is unchanged.
1 parent 295ea9b commit 3e0975b

2 files changed

Lines changed: 94 additions & 98 deletions

File tree

‎frontend/tests/UserProfileRouteTestWrappers.tsx‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import React from "react";
2+
import { UserProfileRoute } from "../src/components/routes/UserProfileRoute";
3+
import {
4+
openedUser,
5+
routeLoading,
6+
routeError,
7+
type OpenedUserProfile,
8+
} from "../src/graphql/cache";
9+
10+
/**
11+
* Wrappers that seed the routing reactive vars before rendering
12+
* UserProfileRoute. Playwright CT mounts run in the browser, so
13+
* `test.beforeEach` callbacks (which run in node) cannot reach the makeVar
14+
* instances the component reads. Setting the vars synchronously in render
15+
* — before the child mounts — gives UserProfileRoute the exact state
16+
* CentralRouteManager would have set in production.
17+
*/
18+
19+
export const UserProfileRouteLoadingWrapper: React.FC = () => {
20+
openedUser(null);
21+
routeLoading(true);
22+
routeError(null);
23+
return <UserProfileRoute />;
24+
};
25+
26+
export const UserProfileRouteResetWrapper: React.FC = () => {
27+
openedUser(null);
28+
routeLoading(false);
29+
routeError(null);
30+
return <UserProfileRoute />;
31+
};
32+
33+
export const UserProfileRouteSeededWrapper: React.FC<{
34+
user: OpenedUserProfile;
35+
}> = ({ user }) => {
36+
openedUser(user);
37+
routeLoading(false);
38+
routeError(null);
39+
return <UserProfileRoute />;
40+
};

‎frontend/tests/user-profile.ct.tsx‎

Lines changed: 54 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,13 @@ import React from "react";
33
import { test, expect } from "./utils/coverage";
44
import { MockedProvider } from "@apollo/client/testing";
55
import { MemoryRouter, Route, Routes } from "react-router-dom";
6-
import { UserProfileRoute } from "../src/components/routes/UserProfileRoute";
76
import { UserProfile } from "../src/views/UserProfile";
8-
import { GET_USER, GET_USER_BADGES } from "../src/graphql/queries";
7+
import {
8+
UserProfileRouteLoadingWrapper,
9+
UserProfileRouteResetWrapper,
10+
UserProfileRouteSeededWrapper,
11+
} from "./UserProfileRouteTestWrappers";
12+
import { GET_USER_BADGES } from "../src/graphql/queries";
913
import { docScreenshot, releaseScreenshot } from "./utils/docScreenshot";
1014

1115
// Mock user data
@@ -25,145 +29,97 @@ const mockPublicUser = {
2529
totalDocumentsUploaded: 10,
2630
};
2731

28-
test.describe("UserProfileRoute - Hook Ordering Regression (Issue #1295)", () => {
29-
test("mounts with slug under a Routes tree that also defines /profile (no 'Rendered more hooks' crash)", async ({
32+
test.describe("UserProfileRoute - State-Driven Rendering", () => {
33+
// UserProfileRoute is a dumb consumer of openedUser / routeLoading /
34+
// routeError, all owned by CentralRouteManager. The wrappers seed those
35+
// reactive vars in the browser context — see UserProfileRouteTestWrappers.
36+
37+
test("renders the loading display when routeLoading is true and no user is resolved", async ({
3038
mount,
3139
page,
3240
}) => {
33-
// This test exercises the scenario that surfaced the original bug: both
34-
// the redirect route (/profile, no slug) and the render route
35-
// (/users/:slug) live in the same <Routes> tree. When useQuery was below
36-
// the `!slug` early return, React could (depending on fiber reuse)
37-
// compare hook call counts across the two renders and throw
38-
// "Rendered more hooks than during the previous render". With the fix,
39-
// useQuery is called unconditionally and skip: !slug gates the network
40-
// request, so the component renders without crashing on either path.
41-
//
42-
// NOTE: This is a narrow regression guard rather than a full reproduction
43-
// of the original crash. Reproducing the exact crash in-process requires
44-
// keeping the same fiber across /profile -> /users/:slug, which React
45-
// Router <Routes> doesn't do (it unmounts the old element when the
46-
// matched path changes). The positive assertions below (loading display
47-
// for the slug path, absence of hook-count errors) prove the hook
48-
// ordering is stable — exactly what the fix guarantees statically.
49-
const consoleErrors: string[] = [];
50-
page.on("console", (msg) => {
51-
if (msg.type() === "error") consoleErrors.push(msg.text());
52-
});
53-
54-
// Delay the mock so the render parks on the loading state, giving us
55-
// positive visible evidence (not just the absence of a crash) that
56-
// useQuery ran under skip:false on the slug path.
57-
const mocks = [
58-
{
59-
request: {
60-
query: GET_USER,
61-
variables: { slug: "publicuser-123" },
62-
},
63-
delay: 5000,
64-
result: {
65-
data: {
66-
userBySlug: mockPublicUser,
67-
},
68-
},
69-
},
70-
];
71-
7241
const component = await mount(
73-
<MockedProvider mocks={mocks} addTypename={false}>
42+
<MockedProvider mocks={[]} addTypename={false}>
7443
<MemoryRouter initialEntries={["/users/publicuser-123"]}>
7544
<Routes>
76-
<Route path="/profile" element={<UserProfileRoute />} />
77-
<Route path="/users/:slug" element={<UserProfileRoute />} />
45+
<Route
46+
path="/users/:slug"
47+
element={<UserProfileRouteLoadingWrapper />}
48+
/>
7849
</Routes>
7950
</MemoryRouter>
8051
</MockedProvider>
8152
);
8253

83-
// Wait for positive DOM evidence that the slug render reached the
84-
// useQuery call (skip:false branch) instead of a silent bailout.
85-
await expect(page.locator("text=Loading profile...")).toBeVisible({
86-
timeout: 10000,
87-
});
88-
89-
const hookErrors = consoleErrors.filter((e) =>
90-
e.includes("Rendered more hooks than during the previous render")
91-
);
92-
expect(hookErrors).toEqual([]);
54+
await expect(page.locator("text=Loading profile...")).toBeVisible();
9355

9456
await component.unmount();
9557
});
96-
});
9758

98-
test.describe("UserProfile View - Loading and Error States", () => {
99-
test("should show loading state while fetching user data", async ({
59+
test("renders the not-found display when no user is resolved", async ({
10060
mount,
10161
page,
10262
}) => {
103-
const mocks = [
104-
{
105-
request: {
106-
query: GET_USER,
107-
variables: { slug: "publicuser-123" },
108-
},
109-
delay: 2000, // Simulate slow network
110-
result: {
111-
data: {
112-
userBySlug: mockPublicUser,
113-
},
114-
},
115-
},
116-
];
117-
11863
const component = await mount(
119-
<MockedProvider mocks={mocks} addTypename={false}>
120-
<MemoryRouter initialEntries={["/users/publicuser-123"]}>
64+
<MockedProvider mocks={[]} addTypename={false}>
65+
<MemoryRouter initialEntries={["/users/nonexistent-user"]}>
12166
<Routes>
122-
<Route path="/users/:slug" element={<UserProfileRoute />} />
67+
<Route
68+
path="/users/:slug"
69+
element={<UserProfileRouteResetWrapper />}
70+
/>
12371
</Routes>
12472
</MemoryRouter>
12573
</MockedProvider>
12674
);
12775

128-
// Check loading spinner is visible
129-
await expect(page.locator("text=Loading profile...")).toBeVisible();
76+
await expect(page.locator("text=User Not Found")).toBeVisible();
13077

13178
await component.unmount();
13279
});
13380

134-
test("should show error message when user not found", async ({
81+
test("renders the resolved profile when openedUser is populated", async ({
13582
mount,
13683
page,
13784
}) => {
138-
const mocks = [
139-
{
140-
request: {
141-
query: GET_USER,
142-
variables: { slug: "nonexistent-user" },
143-
},
144-
result: {
145-
data: {
146-
userBySlug: null,
85+
const badgesMock = {
86+
request: {
87+
query: GET_USER_BADGES,
88+
variables: { userId: "VXNlclR5cGU6MQ==", limit: 100 },
89+
},
90+
result: {
91+
data: {
92+
userBadges: {
93+
edges: [],
94+
pageInfo: {
95+
hasNextPage: false,
96+
hasPreviousPage: false,
97+
startCursor: null,
98+
endCursor: null,
99+
},
147100
},
148101
},
149102
},
150-
];
103+
};
151104

152105
const component = await mount(
153-
<MockedProvider mocks={mocks} addTypename={false}>
154-
<MemoryRouter initialEntries={["/users/nonexistent-user"]}>
106+
<MockedProvider mocks={[badgesMock]} addTypename={false}>
107+
<MemoryRouter initialEntries={["/users/publicuser-123"]}>
155108
<Routes>
156-
<Route path="/users/:slug" element={<UserProfileRoute />} />
109+
<Route
110+
path="/users/:slug"
111+
element={
112+
<UserProfileRouteSeededWrapper user={mockPublicUser as any} />
113+
}
114+
/>
157115
</Routes>
158116
</MemoryRouter>
159117
</MockedProvider>
160118
);
161119

162-
// Wait for query to complete
163-
await page.waitForTimeout(1000);
164-
165-
// Check error message is displayed
166-
await expect(page.locator("text=User not found")).toBeVisible();
120+
await expect(page.locator("text=Public User")).toBeVisible({
121+
timeout: 10000,
122+
});
167123

168124
await component.unmount();
169125
});

0 commit comments

Comments
 (0)