Skip to content
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.

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

Support callable instances #2517

Merged
merged 2 commits into from
Jun 20, 2023
Merged

Support callable instances #2517

merged 2 commits into from
Jun 20, 2023

Conversation

bojavou
Copy link
Contributor

@bojavou bojavou commented May 31, 2023

Purpose (TL;DR) - mandatory

Support callable instances
Closes #2516

Background (Problem in detail)

Enables testing constructors that return functions. #2516 has further description.

Solution - optional

The constructor return value seems to be checked in proxy-invoke.js. This expands the typeof check to include functions.

if (typeof returnValue !== "object" && typeof returnValue !== "function") {
  returnValue = thisValue;
}

This works for my code in a local patched version. Tests are included to verify.

How to verify - mandatory

  1. Check out this branch
  2. npm install
  3. npm test

You can verify manually by running this code from the checkout root. Output should be true.

import sinon from './pkg/sinon-esm.js'
function CallableInstance () { return () => {} }
const SpiedCallableInstance = sinon.spy(CallableInstance)
const callable = new SpiedCallableInstance()
console.log(typeof callable === 'function')
$ nano test.js
true

Checklist for author

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

@codecov
Copy link

codecov bot commented Jun 1, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (05f05ac) 96.00% compared to head (da60bbf) 96.00%.

❗ Current head da60bbf differs from pull request most recent head be30ce0. Consider uploading reports for the commit be30ce0 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2517   +/-   ##
=======================================
  Coverage   96.00%   96.00%           
=======================================
  Files          40       40           
  Lines        1900     1900           
=======================================
  Hits         1824     1824           
  Misses         76       76           
Flag Coverage Δ
unit 96.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/sinon/proxy-invoke.js 97.91% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@fatso83 fatso83 left a comment

Choose a reason for hiding this comment

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

Thanks, this seems nice :)

@fatso83 fatso83 merged commit a79ccae into sinonjs:main Jun 20, 2023
8 checks passed
@bojavou
Copy link
Contributor Author

bojavou commented Jun 20, 2023

Thank you! I'll be glad to switch back to mainline.

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.

Support callable instances
2 participants