feat(testing/unstable): support for stubbing properties

[IgorM867]

This PR adds support for stubbing properties using stub() function which is mentioned in #5848.
Example:

const obj = {
  prop: "foo",
};

const getterStub = stub(obj, "prop", {
  get: function () {
    return "bar";
  },
});

assertEquals(obj.prop, "bar");
assertSpyCalls(getterStub.get, 1);

Note:
I am new to open source and eager to contribute and learn. Apologies in advance if this PR wastes you time.

[codecov]

Codecov Report

Attention: Patch coverage is 99.37888% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.53%. Comparing base (24cd7cf) to head (65d9604).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
testing/unstable_stub.ts	99.20%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6128      +/-   ##
==========================================
+ Coverage   96.52%   96.53%   +0.01%     
==========================================
  Files         536      538       +2     
  Lines       40765    40898     +133     
  Branches     6118     6150      +32     
==========================================
+ Hits        39347    39481     +134     
+ Misses       1374     1373       -1     
  Partials       44       44              

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

[kt3k]

Looks like a good idea to me, but can you move this stub to unstable_stub.ts (and unstable_stub_test.ts) and existing mock.ts untouched?

[IgorM867]

Sorry for the many commits. Just to confirm: did you mean for me to move the new stub implementation to unstable_stub.ts and keep the old version in mock.ts?

I also encountered a lint error: exported symbol is missing JSDoc documentation. This happens because I need to export registerMock and unregisterMock to use them in unstable_stub.ts. Should I add JSDoc comments for these functions? Currently, they look like this:

// deno-lint-ignore no-explicit-any
export function registerMock(spy: Spy<any, any[], any>) {
  const session = getSession();
  session.add(spy);
}
// deno-lint-ignore no-explicit-any
export function unregisterMock(spy: Spy<any, any[], any>) {
  const session = getSession();
  session.delete(spy);
}

[kt3k]

Sorry for the many commits. Just to confirm: did you mean for me to move the new stub implementation to unstable_stub.ts and keep the old version in mock.ts?

Yes, that is correct. The current structure looks mostly good to me.

I also encountered a lint error: exported symbol is missing JSDoc documentation. This happens because I need to export registerMock and unregisterMock to use them in unstable_stub.ts. Should I add JSDoc comments for these functions? Currently, they look like this:

We don't want to expose them as public APIs. Can you move necessary items to some new file like _mock_utils.ts and importing them from both mock.ts (existing one) and unstable_stub.ts?

[IgorM867]

One check failed, but it’s due to unstable_throttle.ts, so I don’t think it’s related to my changes.

[kt3k]

LGTM. Thanks for updating!

[KyleJune]

One minor comment, this says an instance method was restored when it was actually a property in this case. We could probably determine which it is by looking at the type of the value. Also I'd suggest renaming func to prop in this test case since it is stubbing a property rather than a func.

[KyleJune]

I think it would make sense to support an empty property descriptor for when you want it to start with the current value. I think we should also support passing a value option instead of a getter/setter in cases where they want to both spy on and override the value. Without this suggested change, you'd have to create getters and setters like this, where x could be the current value or the value you want to override it with.

Without this suggestion:

const point = new Point(5, 6);
let x = point.x;
const prop = stub(point, "x", {
  get: function () {
    return x;
  },
  set: function (value: number) {
    x = value;
  },
});

With this suggestion when you want to spy without changing the initial value you would do this:

const point = new Point(5, 6);
const prop = stub(point, "x", {});

If possible to get the typing right, it would be even cooler if you could stub a property like you can stub a function, without the third arg.

const point = new Point(5, 6);
const prop = stub(point, "x");

With this suggestion when you want to spy while also changing the initial value:

const point = new Point(5, 6);
const prop = stub(point, "x", { value: 3 });

With this suggestion, all these code samples would be functionally equivalent, where a fake getter/setter are created for the property, where you can make assertions about them. When restored, the property descriptor would be changed back to what it was originally.

[KyleJune]

On this point, I think another improvement that would be cool is to support spying on a property. Where the difference would basically be that at the end, if the original property descriptor had a value instead of a getter/setter, we would be changing the property descriptor to having the value be whatever we last set it to while it was being spied on. While a stub just always returns it back to it's original value.

[KyleJune]

Technically, on the point class, both x and y are values. We are overriding the property descriptor with one that has a getter instead of a value. Same with the other tests. I would describe this, "stub() property value with a getter".

We should get a test case that shows stubbing a property that originally had a getter, setter, or both works too. In all cases, restore should change the property descriptor to what it was before we stubbed it.

[KyleJune]

For that last part, I think the test cases should be updated to first get a copy of the property descriptor for point.x, then at the end after restoring, assert that the property descriptor for point.x matches what it was originally.