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.

Sign up for GitHub

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

Jump to bottom

fix returns does not override call through #2567

Merged
merged 3 commits into from
Nov 1, 2023
Merged

fix returns does not override call through #2567

merged 3 commits into from
Nov 1, 2023

Conversation

rluvaton
Copy link
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
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.

This looks good to me! Fewer lines = yey!

@codecov Codecov
Copy link

codecov bot commented Oct 31, 2023 •
edited
Loading

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
Copy link
Contributor

fatso83 commented Oct 31, 2023

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
Copy link
Contributor Author

Thanks, for some reason it did not run for me

Fixed

@rluvaton
Copy link
Contributor Author

Should I update package version or it's automated?

@fatso83 fatso83 merged commit 5fde5ae into sinonjs:main Nov 1, 2023
7 checks passed
@fatso83
Copy link
Contributor

fatso83 commented Nov 1, 2023

Version numbers are automated.

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

rluvaton commented Nov 1, 2023

thanks, when the release should be?

@fatso83
Copy link
Contributor

fatso83 commented Nov 1, 2023 •
edited
Loading

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
Copy link
Contributor Author

rluvaton commented Nov 1, 2023 •
edited
Loading

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