Skip to content

Feat async extended matchers #7213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lowlighter wants to merge 3 commits into
base: main
Choose a base branch
from
+59 −7
Open

Feat async extended matchers #7213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8da08ef
feat(expect): support async extended matchers
lowlighter Aug 28, 2025
34282b7
Merge branch 'main' into feat-async-extended-matchers
kt3k Aug 28, 2025
64c2974
Merge branch 'main' into feat-async-extended-matchers
lowlighter Jul 5, 2026
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
38 changes: 37 additions & 1 deletion expect/_extend_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@

import { expect } from "./expect.ts";
import type { Async, Expected, MatcherContext, Tester } from "./_types.ts";
import { AssertionError, assertThrows } from "@std/assert";
import { AssertionError, assertRejects, assertThrows } from "@std/assert";

declare module "./_types.ts" {
interface Expected {
toEqualBook: (expected: unknown) => ExtendMatchResult;
toBeResolved: () => Promise<ExtendMatchResult>;
}
}

Expand Down Expand Up @@ -78,6 +79,24 @@ expect.extend({
pass: result,
};
},
async toBeResolved(context) {
if (!(context.value instanceof Promise)) {
throw new TypeError("Expected value to be a promise");
}
const test = new Promise<void>((resolve) => setTimeout(resolve, 0));
const status = await Promise.race([
context.value.then(() => "resolved"),
test.then(() => "pending"),
]);
await test;
return {
message: () =>
`Expected promise to be ${
{ pending: "resolved", resolved: "pending" }[status]
} (got ${status})`,
pass: status === "resolved",
};
},
});

Deno.test("expect.extend() api test case", () => {
Expand Down Expand Up @@ -158,3 +177,20 @@ Deno.test("expect.extend() example is valid", async () => {
myexpect("foo").not.toBeNull();
myexpect.anything;
});

Deno.test("expect.extend() api test case", async () => {

@bartlomieju bartlomieju Jul 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test name is byte-identical to the existing one at line 83, so the two are indistinguishable in test output — and deno test --filter will match both. Something like "expect.extend() supports async matchers" would describe what it actually covers.

While you're here, three cases worth adding: an async matcher under .resolves/.rejects (the isPromised path should flatten it, but nothing pins that), the expect.assertions() interaction noted above, and a matcher whose promise rejects rather than resolving to pass: false.

const { promise, resolve } = Promise.withResolvers<void>();
await expect(promise).not.toBeResolved();
await assertRejects(
() => expect(promise).toBeResolved(),
AssertionError,
"Expected promise to be resolved (got pending)",
);
resolve();
await expect(promise).toBeResolved();
await assertRejects(
() => expect(promise).not.toBeResolved(),
AssertionError,
"Expected promise to be pending (got resolved)",
);
});
2 changes: 1 addition & 1 deletion expect/_types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export interface MatcherContext {
export type Matcher = (
context: MatcherContext,
...args: any[]
) => MatchResult | ExtendMatchResult;
) => MatchResult | ExtendMatchResult | Promise<ExtendMatchResult>;

@bartlomieju bartlomieju Jul 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where the missing-await hazard originates, and I think it needs addressing before the feature ships.

await expect(p).toBeResolved() works, but expect(p).toBeResolved() without the await silently passes — the returned promise is dropped and any AssertionError inside it becomes an unhandled rejection at best. A test that asserts nothing but reports success is the failure mode a test library most needs to avoid.

The type system doesn't help here either: the call-site signature comes entirely from the user's own declare module augmentation, so there's nothing for a no-floating-promises lint to catch unless the user happens to declare it correctly. The test in this PR declares toBeResolved: () => Promise<ExtendMatchResult>, which isn't what the call actually resolves to — it resolves to void — so even the example gets this subtly wrong.

At minimum, expect.extend's JSDoc needs to state that a matcher may return a promise and that the resulting expectation must be awaited. Better would be documenting the correct augmentation shape (() => Promise<void>) so users get lint coverage for free.


export type Matchers = {
[key: string]: Matcher;
Expand Down
26 changes: 21 additions & 5 deletions expect/expect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,13 +219,29 @@ export function expect<T extends Expected = Expected>(
context.isNot = true;
}
if (name in extendMatchers) {
const result = matcher(context, ...args) as ExtendMatchResult;
if (context.isNot) {
if (result.pass) {
const result = matcher(context, ...args) as
| ExtendMatchResult
| Promise<ExtendMatchResult>;

if (result instanceof Promise) {

@bartlomieju bartlomieju Jul 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instanceof Promise is too narrow, and the failure is silent rather than loud.

A non-native thenable — a userland promise implementation, a Promise from another realm/worker, or anything with a .then — fails this check and falls into the sync branch. There, result.pass is undefined, so the assertion inverts: it always throws for a plain expectation and always passes under .not. A matcher that silently passes is the worst outcome for a test library.

This module already has isPromiseLike for exactly this; using it here also makes the behaviour consistent with how .resolves/.rejects decide what counts as a promise.

return result.then((result) => {

@bartlomieju bartlomieju Jul 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth extracting the shared body rather than duplicating it. This block and the sync branch below are verbatim identical — same isNot/pass logic, same AssertionError, same emitAssertionTrigger(). Two copies of assertion-throwing logic will drift, and a fix applied to one won't obviously need applying to the other.

Something like a local check(result) called from both branches keeps them honest.

Separately, note that moving emitAssertionTrigger() into a .then() means it now fires a microtask later than in the sync path — so an un-awaited async matcher doesn't just pass silently, it also undercounts against expect.assertions(). Worth a test either way, since expect.assertions() is precisely the mechanism a user would reach for to catch a forgotten await.

if (context.isNot) {
if (result.pass) {
throw new AssertionError(result.message());
}
} else if (!result.pass) {
throw new AssertionError(result.message());
}
emitAssertionTrigger();
});
} else {
if (context.isNot) {
if (result.pass) {
throw new AssertionError(result.message());
}
} else if (!result.pass) {
throw new AssertionError(result.message());
}
} else if (!result.pass) {
throw new AssertionError(result.message());
}
} else {
matcher(context, ...args);
Expand Down
Loading