test_runner: support object property mocking

[idango10]

Fixes: #58322

---

This PR adds object property mocking to the test runner.

Example usage:

test('mocks a property value', (t) => {
  const obj = { foo: 42 };
  const prop = t.mock.property(obj, 'foo', 100);

  assert.strictEqual(obj.foo, 100);
  assert.strictEqual(prop.mock.accessCount(), 1);
  assert.strictEqual(prop.mock.accesses[0].type, 'get');
  assert.strictEqual(prop.mock.accesses[0].value, 100);

  obj.foo = 200;
  assert.strictEqual(prop.mock.accessCount(), 2);
  assert.strictEqual(prop.mock.accesses[1].type, 'set');
  assert.strictEqual(prop.mock.accesses[1].value, 200);

  prop.mock.restore();
  assert.strictEqual(obj.foo, 42);
});

[nodejs-github-bot]

Review requested:

• @nodejs/test_runner

[codecov]

Codecov Report

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

Project coverage is 90.22%. Comparing base (d96d57d) to head (e1d143f).
Report is 135 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/test_runner/mock/mock.js	99.38%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #58438      +/-   ##
==========================================
+ Coverage   90.20%   90.22%   +0.01%     
==========================================
  Files         635      635              
  Lines      187174   187774     +600     
  Branches    36751    36866     +115     
==========================================
+ Hits       168846   169421     +575     
- Misses      11120    11126       +6     
- Partials     7208     7227      +19     

Files with missing lines	Coverage Δ
lib/internal/test_runner/mock/mock.js	99.28% <99.38%> (+0.02%)	⬆️

... and 76 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cjihrig]

Thanks for working on this. I think this is a good start, but I'd like to see it more unified with the rest of the mocking framework (particularly mock.method() since the use case is so similar)

[idango10]

Thanks for working on this. I think this is a good start, but I'd like to see it more unified with the rest of the mocking framework (particularly mock.method() since the use case is so similar)

Thanks for the thorough review! 😃
I think I've addressed all your comments and pushed the changes. Let me know if there's anything else you'd like me to tweak.

[benjamingr]

Hey, welcome @idango10 nice seeing you around here, I trust Colin and agree with his feedback.

[cjihrig]

This is looking pretty good to me. Two notes:

1. It doesn't appear that a mockImplementationOnce() equivalent is implemented.
2. Can we add a test for explicitly mocking a property as undefined. This will definitely come up eventually, and I don't think the current implementation can support it. It might take a little bit of work to distinguish between the "explicit undefined" and "use the previously existing value" use cases.

[idango10]

This is looking pretty good to me. Two notes:

1. It doesn't appear that a mockImplementationOnce() equivalent is implemented.
2. Can we add a test for explicitly mocking a property as undefined. This will definitely come up eventually, and I don't think the current implementation can support it. It might take a little bit of work to distinguish between the "explicit undefined" and "use the previously existing value" use cases.

1. I'm wondering about how we should implement this. It's pretty problematic since accesses includes both get and set actions. For gets we have no problem, but set is a different case. If a set happens at the same access index, it will consume the "once" value and change the mocked property to the value we only intended to use once.
2. Great catch, it was indeed a bug. Fixed and added a test for it.

[cjihrig]

I'm wondering about how we should implement this. It's pretty problematic since accesses includes both get and set actions. For gets we have no problem, but set is a different case. If a set happens at the same access index, it will consume the "once" value and change the mocked property to the value we only intended to use once.

Personally, I would just let it apply to both sets and gets and document the caveat you mentioned here. We could make it only apply to gets, but that would be inconsistent with how the rest of the implementation works and we would still need to document that caveat.

[idango10]

I'm wondering about how we should implement this. It's pretty problematic since accesses includes both get and set actions. For gets we have no problem, but set is a different case. If a set happens at the same access index, it will consume the "once" value and change the mocked property to the value we only intended to use once.

Personally, I would just let it apply to both sets and gets and document the caveat you mentioned here. We could make it only apply to gets, but that would be inconsistent with how the rest of the implementation works and we would still need to document that caveat.

I added mockImplementationOnce() which applies to both sets and gets, as you suggested. Please let me know if there's anything else I should change 😃

[regseb]

Suggested change

	for `object[valueName]`. **Default:** The original property value.
	for `object[propertyName]`. **Default:** The original property value.

[idango10]

Hey @cjihrig, hope you're doing well! Sorry to ping you, but I wanted to check if you've had a chance to look at my latest changes. Please let me know if anything needs tweaking. Thanks a lot!

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/67284/

[ljharb]

it seems like the API just takes a value - what about getters/setters, enumerability, writability?

[github-actions]

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @atlowChemi.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

[peteduel]

it seems like the API just takes a value - what about getters/setters, enumerability, writability?

Mocking a value for testing why would modification of property descriptors be desirable?

[nodejs-github-bot]

Landed in 905a722

[ljharb]

@peteduel because enumerability and writability are all things that should be tested?