Sinon 15.0.2 update seems to break `callThrough` functionality #2501

migg24 · 2023-03-24T10:24:56Z

Describe the bug
Updating sinon from 15.0.1 to 15.0.2 breaks callThrough functionality.

To Reproduce
Steps to reproduce the behavior:

Check 15.0.1 behaviour in min reproduction example:
https://stackblitz.com/edit/node-cg7mbz?file=index.spec.js&view=editor
Execute npm test in terminal.

Check 15.0.2 behaviour in min reproduction example:
https://stackblitz.com/edit/node-k36ehn?file=index.spec.js&view=editor
Execute npm test in terminal.

Expected behavior
callThrough should work and in 15.0.2 example the same line should be logged.

Screenshots
Working in 15.0.1:

Not working in 15.0.2:

Context (please complete the following information):

Library version: 15.0.2
Environment: node.js LTS
Example URL: https://stackblitz.com/edit/node-k36ehn?file=index.spec.js&view=editor
Other libraries you are using: jest, jest-sinon

Additional context

The text was updated successfully, but these errors were encountered:

fatso83 · 2023-03-24T13:27:01Z

Both of these fail with the same maximum call stack error

fatso83 · 2023-03-24T13:37:07Z

Just deleted my previous comment about not being able to reproduce

I first failed to reproduce the issue, as I copied the verbatim code, but they used non-absolute version numbers and so resultet in 15.0.2 being used in both variations. "^15.0.1" will include patch releases, whereas "15.0.1" will never move.

So verified!

fatso83 · 2023-03-24T13:54:54Z

Works in version 14, 15.0.0, 15.0.1, but broken in 15.0.2. Probably by my doing, if I am not mistaken 😢 Means we have holes in our test coverage!

Verification testcase that is runnable based on yours, but simplified

https://stackblitz.com/edit/node-3cmnrm (not runnable for the same reasons as above, getting that stack issue)

https://runkit.com/fatso83/sinonjs-sinon-2501 (runnable, and breaking on 15.0.2)

$ npm i sinon@"15.0.0" &&  npx jest

added 2 packages, removed 1 package, changed 1 package, and audited 291 packages in 892ms

32 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
  console.log
    val1:  TestService:doSomething:hei

      at Object.log (index.spec.js:17:13)

  console.log
    val2:  undefined

      at Object.log (index.spec.js:18:13)

 PASS  ./index.spec.js
  sinon breaking change
    ✓ should call through (22 ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        0.309 s, estimated 1 s
Ran all test suites.

carlerik at FAT-WORKSTATION in ~/dev/repro
$ npm i sinon@"15.0.1" &&  npx jest

removed 2 packages, changed 1 package, and audited 289 packages in 732ms

32 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
  console.log
    val1:  TestService:doSomething:hei

      at Object.log (index.spec.js:17:13)

  console.log
    val2:  undefined

      at Object.log (index.spec.js:18:13)

 PASS  ./index.spec.js
  sinon breaking change
    ✓ should call through (19 ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        0.267 s, estimated 1 s
Ran all test suites.

carlerik at FAT-WORKSTATION in ~/dev/repro
$ npm i sinon@"15.0.2" &&  npx jest

added 1 package, changed 1 package, and audited 290 packages in 1s

32 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
  console.log
    val1:  undefined

      at Object.log (index.spec.js:17:13)

  console.log
    val2:  undefined

      at Object.log (index.spec.js:18:13)

 FAIL  ./index.spec.js
  sinon breaking change
    ✕ should call through (21 ms)

  ● sinon breaking change › should call through

    expect(received).toEqual(expected) // deep equality

    Expected: "TestService:doSomething:test1"
    Received: undefined

      17 |     console.log("val1: ", stubTestService.doSomething("hei"));
      18 |     console.log("val2: ", stubTestService.doSomethingElse("hei"));
    > 19 |     expect(stubTestService.doSomething("test1")).toEqual(
         |                                                  ^
      20 |       "TestService:doSomething:test1"
      21 |     );
      22 |     expect(stubTestService.doSomethingElse("test2")).toEqual(undefined);

      at Object.toEqual (index.spec.js:19:50)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        0.289 s, estimated 1 s
Ran all test suites.

fatso83 · 2023-03-24T15:42:46Z

verified issue with git bisect to be commit 19bd99f

$ git bisect bad 
You need to start by "git bisect start"

Do you want me to do it for you [Y/n]? y
status: waiting for both good and bad commits
status: waiting for good commit(s), bad commit known

carlerik at diffia-kraftkar in ~/code/sinon (main|BISECTING)
$ git checkout v15.0.0
Note: switching to 'v15.0.0'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 35652f37 15.0.0

carlerik at diffia-kraftkar in ~/code/sinon ((v15.0.0)|BISECTING)
$ npx mocha repro.test.cjs 


  sinon breaking change
val1:  TestService:doSomething:hei
val2:  undefined
    ✔ should call through


  1 passing (21ms)


carlerik at diffia-kraftkar in ~/code/sinon ((v15.0.0)|BISECTING)
$ git bisect good
Bisecting: 5 revisions left to test after this (roughly 3 steps)
[45be60f3c6afc350eacbceed77539f437a9bbbce] Replace probot/stale with official stale action

carlerik at diffia-kraftkar in ~/code/sinon ((45be60f...)|BISECTING)
$ git bisect run npx mocha repro.test.cjs 
running  'npx' 'mocha' 'repro.test.cjs'


  sinon breaking change
val1:  TestService:doSomething:hei
val2:  undefined
    ✔ should call through


  1 passing (22ms)

Bisecting: 2 revisions left to test after this (roughly 2 steps)
[8663ffa056d3c58e82fa203801d58d3fce3c14a7] Upgrade deps (#2498)
running  'npx' 'mocha' 'repro.test.cjs'


  sinon breaking change
val1:  TestService:doSomething:hei
val2:  undefined
    ✔ should call through


  1 passing (20ms)

Bisecting: 0 revisions left to test after this (roughly 1 step)
[7838b57aef902dc3cd020caeb6162be7f5d043ac] 15.0.2
running  'npx' 'mocha' 'repro.test.cjs'


  sinon breaking change
val1:  undefined
val2:  undefined
    1) should call through


  0 passing (24ms)
  1 failing

  1) sinon breaking change
       should call through:
     AssertionError: [assert.equals] undefined expected to be equal to 'TestService:doSomething:test1'
      at Object.fail (node_modules/@sinonjs/referee/lib/create-fail.js:5:21)
      at Object.fail (node_modules/@sinonjs/referee/lib/define-assertion/index.js:47:17)
      at assertion (node_modules/@sinonjs/referee/lib/define-assertion/index.js:65:11)
      at Function.referee.<computed>.<computed> [as equals] (node_modules/@sinonjs/referee/lib/define-assertion/index.js:92:22)
      at Context.<anonymous> (repro.test.cjs:21:16)
      at processImmediate (node:internal/timers:466:21)



Bisecting: 0 revisions left to test after this (roughly 0 steps)
[19bd99f364ab44f0e2715571e5deab580d9aa7fd] Use no-op for every function when restoring instances (#2499)
running  'npx' 'mocha' 'repro.test.cjs'


  sinon breaking change
val1:  undefined
val2:  undefined
    1) should call through


  0 passing (22ms)
  1 failing

  1) sinon breaking change
       should call through:
     AssertionError: [assert.equals] undefined expected to be equal to 'TestService:doSomething:test1'
      at Object.fail (node_modules/@sinonjs/referee/lib/create-fail.js:5:21)
      at Object.fail (node_modules/@sinonjs/referee/lib/define-assertion/index.js:47:17)
      at assertion (node_modules/@sinonjs/referee/lib/define-assertion/index.js:65:11)
      at Function.referee.<computed>.<computed> [as equals] (node_modules/@sinonjs/referee/lib/define-assertion/index.js:92:22)
      at Context.<anonymous> (repro.test.cjs:21:16)
      at processImmediate (node:internal/timers:466:21)



19bd99f364ab44f0e2715571e5deab580d9aa7fd is the first bad commit
commit 19bd99f364ab44f0e2715571e5deab580d9aa7fd
Author: Carl-Erik Kopseng <carlerik@gmail.com>
Date:   Sun Mar 12 22:08:29 2023 +0100

    Use no-op for every function when restoring instances (#2499)
    
    * issue-2477 Use no-op for every function when restoring instances
    * Change wording to not depend on mutator type in tests

 lib/sinon/stub.js                  |  7 ++++++-
 lib/sinon/util/core/walk-object.js | 20 +++++++++++++++-----
 test/restore-object-test.js        |  5 ++++-
 test/sandbox-test.js               | 19 +++++++++++++++++++
 test/stub-test.js                  |  5 ++++-
 test/util/core/walk-object-test.js |  4 ++--
 6 files changed, 50 insertions(+), 10 deletions(-)
bisect found first bad commit

…ances refs #2477 refs #2501

migg24 · 2023-03-27T08:29:45Z

Thank you very much! Can confirm that the issue is resolved in 15.0.3.

Omission from development :-/ refs #2501 refs #2503

fatso83 added the Bug label Mar 24, 2023

fatso83 mentioned this issue Mar 24, 2023

Using restore after createStubInstance creates frankenstein objects #2477

Closed

fatso83 added a commit that referenced this issue Mar 24, 2023

fix: make it possible to call through to underlying stub in stub inst…

7c42347

…ances refs #2477 refs #2501

fatso83 mentioned this issue Mar 24, 2023

fix: make it possible to call through to underlying stub in stub instance #2503

Merged

2 tasks

fatso83 added a commit that referenced this issue Mar 24, 2023

fix: make it possible to call through to underlying stub in stub inst…

70bfa38

…ances refs #2477 refs #2501

fatso83 closed this as completed in 477064b Mar 26, 2023

fatso83 added a commit that referenced this issue Mar 27, 2023

fix: uncomment test assertions

64f31b6

Omission from development :-/ refs #2501 refs #2503

data-sync-user mentioned this issue Oct 12, 2023

chore(deps-dev): bump sinon from 15.0.2 to 15.0.3 mozilla/addons-linter#4778

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sinon 15.0.2 update seems to break `callThrough` functionality #2501

Sinon 15.0.2 update seems to break `callThrough` functionality #2501

migg24 commented Mar 24, 2023

fatso83 commented Mar 24, 2023 •

edited

fatso83 commented Mar 24, 2023

fatso83 commented Mar 24, 2023 •

edited

fatso83 commented Mar 24, 2023

migg24 commented Mar 27, 2023

Sinon 15.0.2 update seems to break callThrough functionality #2501

Sinon 15.0.2 update seems to break callThrough functionality #2501

Comments

migg24 commented Mar 24, 2023

fatso83 commented Mar 24, 2023 • edited

fatso83 commented Mar 24, 2023

fatso83 commented Mar 24, 2023 • edited

fatso83 commented Mar 24, 2023

migg24 commented Mar 27, 2023

Sinon 15.0.2 update seems to break `callThrough` functionality #2501

Sinon 15.0.2 update seems to break `callThrough` functionality #2501

fatso83 commented Mar 24, 2023 •

edited

fatso83 commented Mar 24, 2023 •

edited