Skip to content

fix: distinguish withArgs configurations from matcher results - #2783

Open
xiehuanyi wants to merge 1 commit into
sinonjs:mainfrom
xiehuanyi:fix/withargs-configuration-equality
Open

xiehuanyi wants to merge 1 commit into
sinonjs:mainfrom
xiehuanyi:fix/withargs-configuration-equality

Conversation

@xiehuanyi

Copy link
Copy Markdown

Purpose (TL;DR)

Addresses #1572. Configuring a literal withArgs stub after a broader matcher currently mutates the matcher fake, so calls with other arguments receive the literal's behavior. This also affects reset and filtered spy history.

Require equality in both directions before reusing a configuration. Use the existing public comparator and retain the matcher-aware invocation path. The extra comparison runs only while registering a forward-matched configuration.

How to verify

  1. Install dependencies with npm ci.
  2. Run the eight regressions in test/src/with-args-config-test.js. Four fail before the fix; all eight pass afterward. They cover nested/crossing patterns, matcher identity, cycles, reset, predicate calls/exceptions, and spy history.
  3. Run the Node, browser and worker suites, lint, formatting, coverage, and build/package contract checks. Locally: 1,568 Node tests pass (12 existing pending); 1,561 actual Chromium tests pass (13 existing pending); the worker assessment passes. Coverage thresholds and CJS/ESM/browser package contracts pass.

The local Ubuntu host requires the repository driver's CI mode for Chromium sandbox settings. Browser tests ran with CI=1 and PUPPETEER_EXECUTABLE_PATH pointing to Chrome 149. Lint retains the existing Browserslist notice; dependency checking retains five existing Knip hints.

Authored with Codex assistance; the reported failure was reproduced and the submitted source was tested locally.

Checklist for author

  • npm run lint passes
  • References to standard library functions are cached

Copilot AI balanced review requested due to automatic review settings October 12, 2026 11:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants