test: add authentication library tests by jakebromberg · Pull Request #148 · WXYC/dj-site

jakebromberg · 2026-02-05T17:20:31Z

Closes #248

Summary

Add tests for auth client configuration and getJWTToken / getBaseURL behavior
Add tests for server-client configuration and header-based URL construction
Add focused test files for server-side auth: session management, RBAC, user completeness, and user mapping
Add tests for auth type guards (isAuthenticated, isIncomplete, isPasswordReset) and mapRoleToAuthorization
Add tests for auth utilities (betterAuthSessionToAuthenticationData, toUserFromBetterAuthJWT)
Add tests for session management (authentication slice)
Add centralized BetterAuth session fixtures to test-utils

Non-test changes

Remove Cognito-era exports (djAttributeNames, modifiableAttributeNames) from types.ts
Remove dead Cognito import from authenticationHooks.ts
Delete obsolete api.test.ts (source file was already removed)

Follow-up

Verify-email route testing tracked separately

Test plan

npx vitest run lib/__tests__/features/authentication/ lib/__tests__/features/session.test.ts -- 192 tests passing across 10 files

Part 3 of 26

Note: E2E tests depend on #271 and WXYC/Backend-Service#223. E2E will pass once those merge.

Note: Depends on #146 (test utilities) — both modify lib/test-utils/fixtures.ts. Merge #146 first.

cloudflare-workers-and-pages · 2026-02-05T21:26:23Z

Deploying wxyc-dj with Cloudflare Pages

Latest commit:	`0b5c713`
Status:	🚫 Build failed.

View logs

JacksonMeade

Some of the tests here are focused, clean and nice. Others, I feel, are testing the better-auth library more than our own usage of it, and that is not our responsibility to test. There are also some nits about naming and file separation, and I wish the PR gave a more comprehensive overview of the changes made here, especially where files have been modified.

JacksonMeade · 2026-02-08T19:08:06Z

+
+  afterEach(() => {
+    process.env = originalEnv;
+  });


I'm confused by this environment switching - what does this do? It appears to re-serialize the environment into itself

Agreed, this was confusing. The process.env = { ...originalEnv } in beforeEach was unnecessary since afterEach already restores it. Removed in c4a99a3.

JacksonMeade · 2026-02-08T19:15:56Z

+          ]),
+        })
+      );
+    });


These tests appear to be testing better-auth's behavior, not our implementations of it. Are these tests necessary?

i.e., shouldn't we have tests that verify that the necessary plugins for the backend are present in the actual client that gets created, rather than one that gets mocked?

Agreed. Replaced the mock-shape tests (authClient export checks, plugin factory call assertions, fetchOptions verification) with a single test that verifies the captured config has the expected structure: 3 plugins, credentials include, and a defined baseURL. The getJWTToken and getBaseURL tests stay since those exercise our actual logic.

Good point. The new single config test does exactly this - it captures the config object passed to createAuthClient and asserts on the plugin list, credentials, and baseURL. It verifies what we configure, not what better-auth does with it.

JacksonMeade · 2026-02-08T19:47:30Z

+      expect(capturedConfig?.baseURL).toBe("http://localhost:3001/auth");
+    });
+  });
+});


In my opinion, these tests need to be reevaluated for whether they test the library or the code. Further, the most breakable part of our implementation with the server-client is the verify-email route, which requires special session forwarding due to the JWT + admin client disconnect on using baseUrl. So we should simplify the client tests to be better scoped to our usages of those clients.

Done. Removed the mock-shape blocks (serverAuthClient export, plugin configuration, weak base URL configuration). Replaced with a single config verification test. Kept the getBaseURL header-based URL construction tests since those exercise our actual branching logic. Verify-email route testing is noted as a follow-up in the updated PR description.

JacksonMeade · 2026-02-08T19:49:54Z

+      expect(mockRedirect).not.toHaveBeenCalled();
+    });
+  });
+});


These tests are, I think, the real meat of what needs to be tested in the role system. They don't so much test server utilities as role-based-access-control and good session carryover. They also rely on mocking, while I think we need to really look at redirect behavior. These tests are good, but should be better separated out into different files that describe their actual tested behavior.

Agreed on all counts. Split into four focused files: server-session.test.ts (getServerSession + requireAuth), rbac.test.ts (checkRole + requireRole), user-completeness.test.ts (isUserIncomplete + getIncompleteUserAttributes), and user-mapping.test.ts (getUserFromSession). The file names now describe the tested behavior rather than the source module.

JacksonMeade · 2026-02-08T19:50:21Z

+      const data: AuthenticatedUser = {
+        user: {
+          username: "testuser",
+          email: "test@example.com",


These tests should rely on the central repo of mock users rather than being on their own.

Done. All inline mock user objects replaced with createTestAuthenticatedUser(), createTestIncompleteUser(), and createTestPasswordResetUser() from test-utils.

JacksonMeade · 2026-02-08T19:51:00Z

+      ])('should map "%s" to SM (full access)', (role, expected) => {
+        expect(mapRoleToAuthorization(role)).toBe(expected);
+      });
+    });


This is clean

JacksonMeade · 2026-02-08T19:51:49Z

+    });
+
+    it("should map 'custom:dj-name' to 'djName'", () => {
+      expect(djAttributeNames["custom:dj-name"]).toBe("djName");


This expectation carries over from cognito and is no longer relevant. So I think this points to a need to perform a clean-up re:cognito-based code.

Removed the djAttributeNames and modifiableAttributeNames tests, and also removed those Cognito-era exports from types.ts itself along with the dead import in authenticationHooks.ts.

JacksonMeade · 2026-02-08T19:53:11Z

-      expect(result).toEqual({});
+      expect(result.authority).toBe(Authorization.NO);
    });
  });


This cleans up this code very nicely and tests real functionality. However, I wish the PR description had better informed me of these changes.

Fair point. Updated the PR description to call out the non-test file changes (Cognito removal from types.ts and authenticationHooks.ts, api.test.ts deletion) and note verify-email route testing as a follow-up.

JacksonMeade · 2026-02-08T19:54:08Z

+      expect(runtime).toBe("edge");
+    });
+  });
+});


Remove mock-shape tests that only verify mocks return what they were told. Replace with single configuration verification tests using captured config objects. Split monolithic server-utils.test.ts (705 lines) into four focused files: server-session, rbac, user-completeness, and user-mapping. Remove Cognito-era exports (djAttributeNames, modifiableAttributeNames) from types.ts and dead import from authenticationHooks.ts. Centralize test data using BetterAuth session fixtures in test-utils. Remove over-explaining comments throughout test files.

- Add tests for auth client functionality - Add tests for server-client auth - Add tests for server utilities - Add tests for auth types validation - Add tests for auth utilities - Add tests for session management

Remove mock-shape tests that only verify mocks return what they were told. Replace with single configuration verification tests using captured config objects. Split monolithic server-utils.test.ts (705 lines) into four focused files: server-session, rbac, user-completeness, and user-mapping. Remove Cognito-era exports (djAttributeNames, modifiableAttributeNames) from types.ts and dead import from authenticationHooks.ts. Centralize test data using BetterAuth session fixtures in test-utils. Remove over-explaining comments throughout test files.

- Make checkRole/requireRole/getUserFromSession tests async (these functions use getUserAuthority which fetches from APP_ORGANIZATION) - Add organization-utils mock to rbac and user-mapping tests - Add organizationClient to server-client plugin mock - Update plugin assertions to include organization plugin - Simplify server-client getBaseURL tests to match main's env-only impl

…anch tests

- Fix createTestSessionWithRole to provide all required BetterAuthSession user fields instead of only { role } - Fix session.test.ts: mock react cache function, mock organization-utils and authentication types, replace invalid runtime export test with module exports test - Fix client.test.ts: add vi.resetModules() in beforeEach to prevent JWT token cache leaking between tests, update fetch-count test to clear cache between invocations - Fix server-utils.ts: isUserIncomplete and getIncompleteUserAttributes now check both realName and djName (matching utilities.ts behavior and user-completeness.test.ts expectations) - Update server-utils.test.ts to match corrected djName-checking behavior

Remove mock-shape tests that only verify mocks return what they were told. Replace with single configuration verification tests using captured config objects. Split monolithic server-utils.test.ts (705 lines) into four focused files: server-session, rbac, user-completeness, and user-mapping. Remove Cognito-era exports (djAttributeNames, modifiableAttributeNames) from types.ts and dead import from authenticationHooks.ts. Centralize test data using BetterAuth session fixtures in test-utils. Remove over-explaining comments throughout test files.

jakebromberg force-pushed the test/03-auth-lib branch from cacde0a to 991926c Compare February 5, 2026 21:26

jakebromberg marked this pull request as ready for review February 6, 2026 00:55

jakebromberg requested a review from JacksonMeade February 7, 2026 16:19

JacksonMeade requested changes Feb 8, 2026

View reviewed changes

jakebromberg force-pushed the test/03-auth-lib branch from 02c7259 to 7f3fcdc Compare February 11, 2026 20:09

jakebromberg assigned JacksonMeade Feb 26, 2026

jakebromberg mentioned this pull request Feb 26, 2026

Add comprehensive test suite to dj-site #230

Open

11 tasks

jakebromberg force-pushed the test/03-auth-lib branch from a0eab2a to 1e8aaad Compare February 26, 2026 17:34

jakebromberg force-pushed the test/03-auth-lib branch from 1e8aaad to 6e435dc Compare March 2, 2026 16:07

This was referenced Mar 4, 2026

refactor: use shared validation from @wxyc/shared #123

Merged

test: add test utilities and mock infrastructure #146

Merged

jakebromberg force-pushed the test/03-auth-lib branch from f747ed4 to b79f87f Compare March 9, 2026 01:35

JacksonMeade approved these changes Mar 17, 2026

View reviewed changes

Jake Bromberg added 5 commits March 18, 2026 14:37

test: add authentication library tests

1a84de7

- Add tests for auth client functionality - Add tests for server-client auth - Add tests for server utilities - Add tests for auth types validation - Add tests for auth utilities - Add tests for session management

fix: add createTestSessionWithRole and missing fixture helpers for br…

8ab8c2d

…anch tests

jakebromberg force-pushed the test/03-auth-lib branch from b79f87f to 0b5c713 Compare March 18, 2026 21:38

jakebromberg merged commit f542444 into main Mar 18, 2026
3 of 5 checks passed

jakebromberg deleted the test/03-auth-lib branch March 18, 2026 21:46

Conversation

jakebromberg commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Non-test changes

Follow-up

Test plan

Uh oh!

cloudflare-workers-and-pages Bot commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying wxyc-dj with Cloudflare Pages

Uh oh!

JacksonMeade left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jakebromberg commented Feb 5, 2026 •

edited

Loading

cloudflare-workers-and-pages Bot commented Feb 5, 2026 •

edited

Loading