Skip to content

fix returns does not override call through#2567

Merged
fatso83 merged 3 commits into
sinonjs:mainfrom
rluvaton:override-call-through-on-returns
Nov 1, 2023
Merged

fix returns does not override call through#2567
fatso83 merged 3 commits into
sinonjs:mainfrom
rluvaton:override-call-through-on-returns

Conversation

@rluvaton

@rluvaton rluvaton commented Oct 31, 2023

Copy link
Copy Markdown
Contributor

Purpose (TL;DR) - mandatory

Fix issue #2566 by clearing the fake current behavior

How to verify - mandatory

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

Checklist for author

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

fatso83
fatso83 approved these changes Oct 31, 2023
View reviewed changes

@fatso83 fatso83 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good to me! Fewer lines = yey!

@codecov

codecov Bot commented Oct 31, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1eb070a) 96.05% compared to head (e2fbca5) 96.02%.
Report is 1 commits behind head on main.

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

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2567      +/-   ##
==========================================
- Coverage   96.05%   96.02%   -0.04%     
==========================================
  Files          41       41              
  Lines        1928     1913      -15     
==========================================
- Hits         1852     1837      -15     
  Misses         76       76
Flag Coverage Δ
unit 96.02% <100.00%> (-0.04%) ⬇️

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

Files Coverage Δ
lib/sinon/default-behaviors.js 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fatso83

fatso83 commented Oct 31, 2023

Copy link
Copy Markdown
Contributor

Just run npm run prettier:write to fix the failing test. Usually Prettier will run as a commit hook, unless you explicitly disable those.

@rluvaton

rluvaton commented Oct 31, 2023

Copy link
Copy Markdown
Contributor Author

Thanks, for some reason it did not run for me

Fixed

@rluvaton

rluvaton commented Oct 31, 2023

Copy link
Copy Markdown
Contributor Author

Should I update package version or it's automated?

@fatso83 fatso83 merged commit 5fde5ae into sinonjs:main Nov 1, 2023
@fatso83

fatso83 commented Nov 1, 2023

Copy link
Copy Markdown
Contributor

Version numbers are automated.

@rluvaton rluvaton deleted the override-call-through-on-returns branch November 1, 2023 10:08
@rluvaton

rluvaton commented Nov 1, 2023

Copy link
Copy Markdown
Contributor Author

thanks, when the release should be?

@fatso83

fatso83 commented Nov 1, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

It's already out (just wait a few minutes to clear the cache)

P.S. You do not ever have to wait for me to release a new Sinon version (or anyone else's package). Just use patch-package while you wait. Super good!

@rluvaton

rluvaton commented Nov 1, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Yeah, I love patch package but prefer to avoid it as much as possible

thanks!

@fpascutti fpascutti mentioned this pull request Nov 6, 2023
Closed
fatso83 added a commit that referenced this pull request Apr 25, 2024
@fatso83
Partially revert "fix returns does not override call through (#2567)"
f4fb595 
This partially reverts commit 5fde5ae:
- we keep the tests
- revert to the old manual clearing of props

The clearing of callThrough has then been manually added to what I
deem are the relevant spots. Tests are unaltered.
@fatso83 fatso83 mentioned this pull request Apr 25, 2024
Merged
2 tasks
fatso83 added a commit that referenced this pull request Apr 25, 2024
@fatso83
Avoid return and callArg* clearing each other's state (#2593)
5025d00 
* Add regression test for #2572

* Partially revert "fix returns does not override call through (#2567)"

This partially reverts commit 5fde5ae:
- we keep the tests
- revert to the old manual clearing of props

The clearing of callThrough has then been manually added to what I
deem are the relevant spots. Tests are unaltered.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fatso83 fatso83 fatso83 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rluvaton @fatso83