Skip to content

Conversation

@boneskull
Copy link
Owner

The 'to throw error satisfying', 'to reject with error satisfying', and
similar assertions had special handling for string/RegExp parameters that
bypassed valueToSchema, causing expect.it() functions to fail validation.

Changed the final else-if branch to a plain else clause, letting
valueToSchema handle all non-string/non-RegExp parameters including
ExpectItExecutor functions.

Also expanded property test generators to include expect.it() cases
for better coverage of these assertions.

The 'to throw error satisfying', 'to reject with error satisfying', and
similar assertions had special handling for string/RegExp parameters that
bypassed valueToSchema, causing expect.it() functions to fail validation.

Changed the final else-if branch to a plain else clause, letting
valueToSchema handle all non-string/non-RegExp parameters including
ExpectItExecutor functions.

Also expanded property test generators to include expect.it() cases
for better coverage of these assertions.
Copilot AI review requested due to automatic review settings January 17, 2026 00:27
Copy link
Owner Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where expect.it() functions failed when used within error-satisfying assertions. The root cause was that the special handling for string/RegExp parameters bypassed valueToSchema, which properly handles ExpectItExecutor functions.

Changes:

  • Modified error-satisfying assertions to delegate all non-string/non-RegExp parameters to valueToSchema
  • Removed unnecessary error handling and type guards since valueToSchema always returns a valid schema
  • Expanded test generators to include expect.it() cases for better coverage

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
packages/bupkis/src/assertion/impl/sync-parametric.ts Changed conditional logic from else if (isNonNullObject(param)) to else to allow valueToSchema to handle all parameter types; removed unnecessary error handling code
packages/bupkis/src/assertion/impl/async-parametric.ts Applied same conditional logic fix for async assertions; consistent with sync changes
packages/bupkis/test-data/sync-parametric-generators.ts Added test generators for expect.it() cases in satisfies and error-satisfying assertions
packages/bupkis/test-data/async-parametric-generators.ts Added test generators for expect.it() cases in async error-satisfying assertions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@boneskull boneskull added bug Something isn't working assertions CRUD operations on built-in assertions labels Jan 18, 2026
@boneskull boneskull merged commit 11d485f into main Jan 18, 2026
19 checks passed
@boneskull boneskull deleted the to-satisfy-semantics-fix branch January 18, 2026 01:05
@github-actions github-actions bot mentioned this pull request Jan 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

assertions CRUD operations on built-in assertions bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants