tests: mock better-auth client and assert rejections in branching tests#51
Merged
weroperking merged 1 commit intomainfrom Mar 29, 2026
Merged
Conversation
WalkthroughImport statement reorganized in one test file to place Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/test/branching.test.ts`:
- Around line 398-416: Update the three test titles to match the asserted
failure behavior: change the describe/test names for listPreviewDatabases,
previewDatabaseExists, and teardownPreviewDatabase (the tests calling
dbBranching.listPreviewDatabases(),
dbBranching.previewDatabaseExists("preview_test"), and
dbBranching.teardownPreviewDatabase(...)) so they state they expect a rejection
on connection failure (e.g., "rejects when connection fails" or "throws on
connection error") instead of implying a successful return; alternatively, if
you intended to test the happy path, modify the assertions to await resolved
values rather than using rejects.toThrow().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e4d04716-be8a-4d76-a688-b68debabc0e6
📒 Files selected for processing (2)
packages/client/test/auth.test.tspackages/core/test/branching.test.ts
Comment on lines
398
to
416
| describe("listPreviewDatabases", () => { | ||
| test("returns array of preview database names", async () => { | ||
| // Without actual DB connection, this will fail | ||
| // But we can verify it returns a promise | ||
| const promise = dbBranching.listPreviewDatabases(); | ||
| expect(promise).toBeInstanceOf(Promise); | ||
| await expect(dbBranching.listPreviewDatabases()).rejects.toThrow(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("previewDatabaseExists", () => { | ||
| test("returns promise for checking database existence", async () => { | ||
| const promise = dbBranching.previewDatabaseExists("preview_test"); | ||
| expect(promise).toBeInstanceOf(Promise); | ||
| await expect(dbBranching.previewDatabaseExists("preview_test")).rejects.toThrow(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("teardownPreviewDatabase", () => { | ||
| test("returns promise for teardown operation", async () => { | ||
| const promise = dbBranching.teardownPreviewDatabase( | ||
| "postgres://user:password@localhost:5432/preview_test", | ||
| ); | ||
| expect(promise).toBeInstanceOf(Promise); | ||
| await expect( | ||
| dbBranching.teardownPreviewDatabase("postgres://user:password@localhost:5432/preview_test"), | ||
| ).rejects.toThrow(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Test descriptions don't match test behavior.
The test names describe happy-path behavior ("returns array", "returns promise") but the assertions verify rejection on connection failure. This is misleading when tests fail or when reading the test output.
Suggested renames
describe("listPreviewDatabases", () => {
- test("returns array of preview database names", async () => {
+ test("rejects when database connection fails", async () => {
await expect(dbBranching.listPreviewDatabases()).rejects.toThrow();
});
});
describe("previewDatabaseExists", () => {
- test("returns promise for checking database existence", async () => {
+ test("rejects when database connection fails", async () => {
await expect(dbBranching.previewDatabaseExists("preview_test")).rejects.toThrow();
});
});
describe("teardownPreviewDatabase", () => {
- test("returns promise for teardown operation", async () => {
+ test("rejects when database connection fails", async () => {
await expect(
dbBranching.teardownPreviewDatabase("postgres://user:password@localhost:5432/preview_test"),
).rejects.toThrow();
});
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| describe("listPreviewDatabases", () => { | |
| test("returns array of preview database names", async () => { | |
| // Without actual DB connection, this will fail | |
| // But we can verify it returns a promise | |
| const promise = dbBranching.listPreviewDatabases(); | |
| expect(promise).toBeInstanceOf(Promise); | |
| await expect(dbBranching.listPreviewDatabases()).rejects.toThrow(); | |
| }); | |
| }); | |
| describe("previewDatabaseExists", () => { | |
| test("returns promise for checking database existence", async () => { | |
| const promise = dbBranching.previewDatabaseExists("preview_test"); | |
| expect(promise).toBeInstanceOf(Promise); | |
| await expect(dbBranching.previewDatabaseExists("preview_test")).rejects.toThrow(); | |
| }); | |
| }); | |
| describe("teardownPreviewDatabase", () => { | |
| test("returns promise for teardown operation", async () => { | |
| const promise = dbBranching.teardownPreviewDatabase( | |
| "postgres://user:password@localhost:5432/preview_test", | |
| ); | |
| expect(promise).toBeInstanceOf(Promise); | |
| await expect( | |
| dbBranching.teardownPreviewDatabase("postgres://user:password@localhost:5432/preview_test"), | |
| ).rejects.toThrow(); | |
| }); | |
| }); | |
| describe("listPreviewDatabases", () => { | |
| test("rejects when database connection fails", async () => { | |
| await expect(dbBranching.listPreviewDatabases()).rejects.toThrow(); | |
| }); | |
| }); | |
| describe("previewDatabaseExists", () => { | |
| test("rejects when database connection fails", async () => { | |
| await expect(dbBranching.previewDatabaseExists("preview_test")).rejects.toThrow(); | |
| }); | |
| }); | |
| describe("teardownPreviewDatabase", () => { | |
| test("rejects when database connection fails", async () => { | |
| await expect( | |
| dbBranching.teardownPreviewDatabase("postgres://user:password@localhost:5432/preview_test"), | |
| ).rejects.toThrow(); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/test/branching.test.ts` around lines 398 - 416, Update the
three test titles to match the asserted failure behavior: change the
describe/test names for listPreviewDatabases, previewDatabaseExists, and
teardownPreviewDatabase (the tests calling dbBranching.listPreviewDatabases(),
dbBranching.previewDatabaseExists("preview_test"), and
dbBranching.teardownPreviewDatabase(...)) so they state they expect a rejection
on connection failure (e.g., "rejects when connection fails" or "throws on
connection error") instead of implying a successful return; alternatively, if
you intended to test the happy path, modify the assertions to await resolved
values rather than using rejects.toThrow().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
better-auth/clientimplementation is mocked beforeAuthClientis imported so auth tests run deterministically underbun:test.Promise.Description
bun:testimports to the top ofpackages/client/test/auth.test.tsand registermock.module("better-auth/client", ...)socreateAuthClientand its methods are mocked beforeAuthClientis imported.bun:testimport and keep theMockStoragehelper used by auth tests.expect(...).toBeInstanceOf(Promise)checks inpackages/core/test/branching.test.tswithawait expect(...).rejects.toThrow()forlistPreviewDatabases,previewDatabaseExists, andteardownPreviewDatabaseto assert they reject without a real DB.Testing
bun test packages/client/test/auth.test.tsand they passed.bun test packages/core/test/branching.test.tsand they passed.Codex Task
Summary by CodeRabbit