Closed egmacke closed 2 months ago
Good repro! Thanks.
Do you want to take a stab at it? I am guessing it is not a very hard fix. We have good test coverage.
For this given replit if you print the source for the object whose getter is being replaced(using descriptor.get.toString()), the function has changed to the replacement. The console output you see is coming from line 246 in the function getFakeRestorers when the getter is executed to potentially set the field if forceAssignment is true.
Logically it needs to execute this way because restorers need to resolve the getter to be able to restore the original value you had in that function.
A good example is the test starting on line 1119 in sandbox-test where you have a variable that is being get and set by an object. Even if you try to delay the execution of the getter it will return you the most recent value of that variable, not the past value that needs to be restored. In the case that you just have simple output like a constant ('original' or 'fake') it's obviously different because you could get the descriptor for foo and then use the getter in that descriptor.
From my understanding getters should be pretty simple and should avoid side effects to begin with, so the solution in my opinion here should be to avoid side effects in your getters.
This is the code in question that is a bit too simple:
The simple direct access is the issue.
Since we only need the direct value when forcing use of accessors, we can check for that case. Fix upcoming.
Describe the bug Using
sinon.replaceGetter
correctly replaces the getter, but in the process it appears to invoke the originalget
function.This causes issues when the getter is non-trivial and relies on class state which may not be correctly setup in a test environment.
To Reproduce Steps to reproduce the behavior:
sinon.replaceGetter
Reproduction here: https://replit.com/@egmacke/Sinon-replaceGetter-bug
Expected behavior
Expect that the getter is replaces without invoking the original implementation.
Context (please complete the following information):
Output of
npx envinfo --browsers --binaries
: