search

sinonjs / sinon

Test spies, stubs and mocks for JavaScript.
https://sinonjs.org/
Other
9.63k stars 769 forks source link

fix: actaully reset 'injectedKeys' #2456

Closed mroderick closed 1 year ago

mroderick commented 2 years ago

Re-assigning the local variable injectedKeys would not change sandbox.injectedKeys, thus restoreContext doesn't fully restore the context.

See:

https://lgtm.com/projects/g/sinonjs/sinon/snapshot/9e09e7d79bac5808ca98fac4f7419a20be4fc43d/files/lib/sinon/sandbox.js?sort=name&dir=ASC&mode=heatmap#x9f770d565ef51b7d:1

codecov[bot] commented 2 years ago

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01 :warning:

Comparison is base (3b41aff) 95.99% compared to head (98cffd0) 95.99%.

:exclamation: Current head 98cffd0 differs from pull request most recent head 804d691. Consider uploading reports for the commit 804d691 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2456 +/- ## ========================================== - Coverage 95.99% 95.99% -0.01% ========================================== Files 40 40 Lines 1898 1896 -2 ========================================== - Hits 1822 1820 -2 Misses 76 76 ``` | Flag | Coverage Δ | | |---|---|---| | unit | `95.99% <100.00%> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/sinonjs/sinon/pull/2456?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) | Coverage Δ | | |---|---|---| | [lib/sinon/sandbox.js](https://codecov.io/gh/sinonjs/sinon/pull/2456?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs#diff-bGliL3Npbm9uL3NhbmRib3guanM=) | `97.72% <100.00%> (-0.03%)` | :arrow_down: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs)

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

fatso83 commented 2 years ago

Any way of adding a regression test to show the bug?

mroderick commented 2 years ago

Any way of adding a regression test to show the bug?

That's a fair question. We haven't detected it thus far with tests and users have also not reported it. There might be something else that resets that property, so we've been hiding in the shadow of that for years.

I can look further